-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
Use minimum font-size of 12px #992
Use minimum font-size of 12px #992
Conversation
PageSpeed Insights and Lighthouse complain about text that's smaller than 12px (for mobile). Our styling for `pre code` and `#information .credits` both use a `font-size` that's smaller than 12px (11px and ~11.5281px respectively). This increases the smallest `font-size` values in `style.scss` to 12px (80% in this case).
dce55ea
to
1b4ba70
Compare
@samford Can I see a before/after screenshot for this on desktop/mobile (and generally for anything that changes appearance)? Note that I may suggest this be scoped just to mobile depending on how it looks. |
@samford Ironically: I think the desktop situation is better but mobile is worse after these changes. The install link is not even vaguely readable and the |
I'm saying we hide the entire URL and perhaps show something else instead. It's not useful one mobile. |
@samford, you can target specific screen sizes by placing your changes inside |
If we're generally fine with these
I'm aware (I do web development for a living 😆) but my thinking was that the If we want to scope the |
I'm not, sorry. As-is this is a "make linter happy" change that makes things worse rather than better for users. As before, I'd happily see a redesign of this part of the page that means this isn't needed for install URL, something that's not going to be copy/pasted on an iPhone.
Would you like the idea of what it should look like or the code or both or neither?
We're not really displaying code in the same way as GitHub here, though. |
A concrete idea and any text changes would be good (i.e., I'm fine to take care of any implementation). At the moment, all of the text in the "Install Homebrew" section either references the install snippet or talks about the pkg installer. That said, though mobile users wouldn't have any direct use for the install information on their device, I'm a little wary of assuming that it wouldn't be useful to them in general. They wouldn't be copy/pasting the snippet or using the pkg installer but I'm not totally convinced that the install information isn't useful to them at all. For example, someone may load the homepage for the first time on mobile (e.g., checking it while they're away from their computer after hearing about the project) and the install instructions give them an idea of how |
@samford Cool, I would replace all text in the "Install Homebrew" section (i.e. everything before "What Does Homebrew Do?") with a single line of text saying something like Homebrew's [installation script](https://github.com/Homebrew/install/blob/HEAD/install.sh) is run from a macOS Terminal or Linux shell prompt.
Check this page on your computer for the copy-pasteable command.
Alternatively, read about other [Homebrew's other installation options](https://docs.brew.sh/Installation). I think those two links are the two things people might want to look at and they are fairly accessible on mobile compared to trying to extract anything meaningful from the text string with |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
I'll try to remember to revisit this over the weekend when I have a moment. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
PageSpeed Insights and Lighthouse complain about text that's smaller than 12px (for mobile). Our styling for
pre code
and#information .credits
both use afont-size
that's smaller than 12px (11px and ~11.5281px respectively).This increases the smallest
font-size
values instyle.scss
to 12px (80% in this case).