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

Update Fonts related to SurveySurfaceViewController to use FXFontStyles #20307

Open
data-sync-user opened this issue May 17, 2024 · 5 comments
Open
Labels
Contributor Fix A contributor has fixed this issue. It might still be opened as we're waiting for final QA approval Contributor OK This is a good issue for contributors interested in helping the project Fonts Relates to standardizing Fonts in the application

Comments

@data-sync-user
Copy link
Collaborator

data-sync-user commented May 17, 2024

This task is part of a series of tasks to standardizing fonts to start using FXFontStyles.

Background Context:
Previously, we were using DefaultDynamicFontHelper and setting the text style, size and weight for each font, which sometimes did not match our design system. By using FXFontStyles, we can standardize our fonts and be more aligned with the design system.

Task:
Please update how we set fonts in the Firefox iOS Project related:

SurveySurfaceViewController

Acceptance Criteria:

Replace usage of DefaultDynamicFontHelper with FXFontStyles

If there are discrepancies in terms of replacing the old fonts with the new standard, we will need to bring design in to approve the PR.

Please provide before and after screenshots of the UI.

Reference:

Usage example:
FXFontStyles.Regular.headline.scaledFont()

See this PR for example of standardizing fonts for Primary Button:

https://github.com/mozilla-mobile/firefox-ios/pull/18711/files

┆Issue is synchronized with this Jira Task

@cyndichin cyndichin added Contributor OK This is a good issue for contributors interested in helping the project Fonts Relates to standardizing Fonts in the application labels May 17, 2024
@tisumi99
Copy link
Contributor

I'll work on this one.

@tisumi99
Copy link
Contributor

@cyndichin
I submitted the PR for this but I couldn't figure out how to get the survey to show - so I couldn't get screenshots. Any suggestions on how I can get the survey to show?

FXFontStyle matches the original font size and style so I think there is very low risk that the UI changed.

@cyndichin
Copy link
Contributor

Thanks for picking up this ticket! One way to get it to show is to go to our messaging-evergreen-messages.fml.yaml file.

You should see this section which is an example configuration for the survey message
image.

Change trigger-if-all: from NEVER to ALWAYS and you should see it pop up. Let me know if you have any issues!

@tisumi99
Copy link
Contributor

@cyndichin
That worked. Thanks! Screenshots posted to the PR.

@cyndichin cyndichin added the Contributor Fix A contributor has fixed this issue. It might still be opened as we're waiting for final QA approval label May 20, 2024
@data-sync-user
Copy link
Collaborator Author

➤ Cyndi Chin commented:

QA, please verify that the fonts for the survey view scale appropriately. See screenshots in the PR. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributor Fix A contributor has fixed this issue. It might still be opened as we're waiting for final QA approval Contributor OK This is a good issue for contributors interested in helping the project Fonts Relates to standardizing Fonts in the application
Projects
None yet
Development

No branches or pull requests

3 participants