Refactor font sizes for text content #8052
Conversation
Malvoz
added
accessibility
|
An alternative solution to the changes suggested here is to convert and use |
|
This is great! But the default font sizes look unreasonably large to me, and completely out of character for interactive maps. The font size legibility guidelines make sense for content-heavy web pages, but it's very different here — popups are usually small and have very limited space to afford gigantic default text size, and attribution isn't really meant to be prominent and read by everyone, it's more of a small aside, unobtrusive and not taking attention away from the map itself. |
|
@mourner Thanks for the feedback. I don't share the same opinion (#8051), but you're the man. Feel free to close this PR. @natevw Do you want to send a PR for your proposal for |
|
Maybe we could settle on some kind of a compromise? Still fix relative font sizes and "large" setting, and increase the default size a little bit, but not that much? |
|
I see the benefits for the accessibility but when I think out of my project view and not from Leaflet view, I wouldn't like that the text is after an update to 1.8.0 much bigger especially the attribution control. I add the |
Falke-Design
added
the
blocker
| @@ -431,8 +430,7 @@ svg.leaflet-image-layer.leaflet-interactive path { | |||
| border-top: none; | |||
| line-height: 1.1; | |||
| padding: 2px 5px 1px; | |||
| font-size: 11px; | |||
| font-size: 0.69rem; | |||
| font-size: smaller; | |||
There was a problem hiding this comment.
Also, do we know if relative sizes like smaller work in older IE versions? I'd maybe leave the adjusted pixel definitions before new ones just in case.
There was a problem hiding this comment.
Thanks for pointing this out. I thought about it but it still managed to slip my mind. I did a quick search for browser support but didn't find anything. The lack of support data is usually indicative of legacy features, but unless we find any support data we should keep the px variants as fallback.
Yes, I still have my VM set up for this project and could do so this coming week. Basically my approach would be to replace the new |
|
Closing per #8057:
The "potential problems" were expected and described in this PR, but maybe a big breaking change like this (if accepted) is better suited for v2.0.0. |
Comparison
font-size[NOTE][NOTE]: The custom font sizes used
With these particular custom styles, the computed font-size SHOULD be smaller than the default font-size in the screenshots.
<html>) font size?(by accepting that the new relative font sizes may cause illegible fonts depending on the application's CSS (i.e. responsibility is on the developer to not use too small font sizes), this is simply the nature of relative font sizes. In other words things like v1.8.0-beta issue Tracker #8044 (comment) is expected and some developers may have to make CSS changes, the alternative is to close this PR, revert changes in Use relative
font-sizeto respect the user's font size settings in the browser #7800, and go back to the 1.7.1 scenario)font-sizein the attribution #7346(because the attribution and scale control now has the same font size as the
<small>element due tofont-size: smaller, which is larger than 12px)leaflet-container#8051(because the default font size in Leaflet is now equal to that of body text, except for attribution and scale control which remain smaller)