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

16 Replaced all fonts with Montserrat #196

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

16 Replaced all fonts with Montserrat #196

wants to merge 11 commits into from

Conversation

shashjar
Copy link
Contributor

@shashjar shashjar commented Jun 14, 2022

Changes

Added RegularText, MediumText, and BoldText components, and updated the code to use these. Also merged in dev and handled the conflicts. Fonts are imported and loaded in in App.tsx. There are constants set in constants/fonts.tsx for the strings of each font type, to be used for styling. If we change fonts at any point, we will only have to change the imports and useFonts statement in App.tsx and the declarations in constants/fonts.tsx. Updated fonts in tutorial pictures.

Also edited game screen to switch order of buttons. end session is now on the left and next round is now on the right, to favor right-handed players. The corresponding tutorial image was updated. This was issue #206.

Also fixed the Ananas-Pineapple formatting issue on Test Question 32. This was issue #195.

TODOs

  • Make sure no formatting issues created anywhere in the app

Fonts

  • Montserrat 400Regular for all body text and regular content
  • Montserrat 500Medium for all button text (except for game buttons, which are 700Bold)
  • Montserrat 700Bold for most headers (some exceptions that are 500Medium instead)

Closes #16
Closes #206
Closes #195

@shashjar shashjar changed the base branch from dev to 10-firebase-emulator June 17, 2022 00:40
@shashjar shashjar changed the base branch from 10-firebase-emulator to dev June 17, 2022 00:40
@shashjar shashjar mentioned this pull request Jun 26, 2022
Copy link
Collaborator

@emaela emaela left a comment

Choose a reason for hiding this comment

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

The font looks good to me, except for the bug noted in #195.

However, I have a thought on centralizing the font changes: currently the change is made by updating the styling for each component to include a fontFamily property, but I was thinking that another possibility would be to create custom Text components with the appropriate font styling and then update the screens to use those custom Text components. So we could have something like a BoldText component that is styled with Montserrat_700Bold and so on (perhaps with better naming).
My reasoning for this is that in the (very unlikely) case that we change the font type, size or boldness that we want for a certain type of text in the app (header text, button text, body text), we could change all of that type of text by editing one component file rather than each file that has that type of text.

Of course, this may have drawbacks I haven't considered, what are your thoughts on this?

@emaela
Copy link
Collaborator

emaela commented Jul 15, 2022

Note: we should have edit access to Figma, so the font for tutorial images should be able to be updated

@shashjar
Copy link
Contributor Author

shashjar commented Jul 20, 2022

Added RegularText, MediumText, and BoldText components, and updated the code to use these. Also merged in dev and handled the conflicts.

Still have to update fonts in tutorial pictures.

Settings page looks a little wonky - will have to update this.

Wait for #205 to be merged into dev - then will have to update the fonts in adminScreen.tsx and consentScreen.tsx.

@shashjar shashjar changed the base branch from dev to 12-upload-script July 21, 2022 02:58
@shashjar shashjar changed the base branch from 12-upload-script to dev July 21, 2022 02:58
@shashjar shashjar changed the base branch from dev to 12-upload-script July 21, 2022 23:17
@shashjar shashjar changed the base branch from 12-upload-script to dev July 21, 2022 23:17
Copy link
Collaborator

@emaela emaela left a comment

Choose a reason for hiding this comment

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

Pretty much everything looks good to me, I just noticed:

  • The 'change password' button on the Settings screen is cut off (horizontally), we should make it wider
  • The 'done' button on the Test End screen is cut off (vertically), we should make it longer/adjust padding

IMG_4627
IMG_4628

@shashjar
Copy link
Contributor Author

Fixed the formatting issues for the buttons on the Settings screen and the Test Results screen. I changed the buttons on the Settings screen to be centered because I think it looks better but let me know if you think we should change back to right-aligned.

@Octillerysnacker Octillerysnacker self-requested a review August 12, 2022 17:26
Copy link
Contributor

@Octillerysnacker Octillerysnacker left a comment

Choose a reason for hiding this comment

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

Everything looks good, just one thing. The height of the buttons on the sign in flow change whenever I open my keyboard, is that intentional?
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants