Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong message text font when text accessibility is larger + Snapshot Testing Framework Update #2575

Merged

Conversation

nuno-vieira
Copy link
Member

@nuno-vieira nuno-vieira commented Apr 19, 2023

🔗 Issue Links

Resolves https://github.com/GetStream/ios-issues-tracking/issues/371

🎯 Goal

Fixes the message text font when the text accessibility setting is set to larger.

This issue was introduced by this PR, which we tried to fix the dynamic font size for the message text, but this actually causes the text font to be with an incorrect size after closing/opening the app. So we need to revert that change.

For now, we don't know the root cause of why the font is not changed dynamically, and it only changes after closing the app. But there is a workaround to overcome this, and it is to override the MessageContentView.defaultMessageFont to always return a new UIFont instance (Ex: UIFont.preferredFont(forTextStyle: .body)). More context here.

Bonus: Fixes the message timestamp label and checkmark delivery status issues when the font size is larger.

📝 Summary

  • Fixes the message text font when the text accessibility setting is set to larger.
  • Fixes the timestamp label being clipped when the text font size is larger.
  • Fixes the checkmark in the delivery status view being too small for larger text font sizes.
  • Updates the Snapshot Testing framework to improve M1 and Intel SSs

🎨 Showcase

Before After

🧪 Manual Testing Notes

  1. Xcode > Open Developer Tool > Accessibility Inspector
  2. Processes > Simulator
  3. Tap on Settings button (Right top corner)
  4. Set the Dynamic font to a really big font
  5. Close and open the app
  6. Open a channel
  7. The font of the messages should have the exact size of the Input Placeholder

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change follows zero ⚠️ policy (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@nuno-vieira nuno-vieira added 🐞 Bug An issue or PR related to a bug 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK labels Apr 19, 2023
@nuno-vieira nuno-vieira requested a review from a team as a code owner April 19, 2023 12:06
Copy link
Contributor

@polqf polqf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we end up changing that justifies the change on the snapshots?

@nuno-vieira nuno-vieira force-pushed the fix/message-text-font-wrong-for-large-text-accessibility branch from 4994edd to 6cd190a Compare April 20, 2023 14:10
@nuno-vieira
Copy link
Member Author

Did we end up changing that justifies the change on the snapshots?

Yes, the CheckmarkView now has a different size. It is a little a bit bigger now to improve the UI when text accessibility is bigger

@sonarcloud
Copy link

sonarcloud bot commented Apr 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@nuno-vieira nuno-vieira changed the title Fix wrong message text font when text accessibility is larger Fix wrong message text font when text accessibility is larger + Snapshot Testing Framework Update Apr 20, 2023
CHANGELOG.md Show resolved Hide resolved
@nuno-vieira nuno-vieira merged commit c5b58f6 into develop Apr 21, 2023
16 of 17 checks passed
@nuno-vieira nuno-vieira deleted the fix/message-text-font-wrong-for-large-text-accessibility branch April 21, 2023 09:54
@polqf polqf mentioned this pull request Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants