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

Make 'select all functions' bold #34858

Merged
merged 1 commit into from
May 20, 2020
Merged

Make 'select all functions' bold #34858

merged 1 commit into from
May 20, 2020

Conversation

maddiedierker
Copy link
Contributor

It was difficult to visually differentiate the "Select all functions" input from the function inputs, so making that header bold:
Screen Shot 2020-05-18 at 3 17 57 PM

Links

Testing story

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

selectAllFunctionsLabel: {
margin: 0,
fontSize: 20,
fontFamily: '"Gotham 5r", sans-serif'
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than setting the fontFamily, would it make more sense to set the fontWeight? I'm slightly concerned that explicitly setting the fontFamily could cause this to display as a different looking text font on certain browsers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i changed the fontFamily because that's the convention i've seen throughout apps/ -- are we using fontWeight as the convention instead now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh I didn't realize we bundle Gotham as part of our app. This makes more sense now. It seems a little odd to me that we have sans-serif as a fallback here without making it bold. But I don't think this is a ship blocker for this.

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

LGTM!

@maddiedierker maddiedierker merged commit 18a377d into staging May 20, 2020
@maddiedierker maddiedierker deleted the fix-select-all branch May 20, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants