-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[ios, macos] Adapt Mapbox Streets–sourced layers for Dynamic Type. #9734
[ios, macos] Adapt Mapbox Streets–sourced layers for Dynamic Type. #9734
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we could implement this more generically in Core. There are a few issues with the approach in this PR:
- Doesn't work for layers that use a data-driven size, a zoom-level dependent size, or an expression
- Alters the stylesheet which makes it impossible to revert to the original size
- Implementation logic is specific to iOS/macOS, but Android/Qt/Node could benefit from this too.
Our implementation keeps a reference to all existing values before manipulating it which allows us to revert to the original value and to listen for changes. It also works for zoom level driven styles and should be easy enough to extend to support data driven style values too: |
I would also make this behavior opt-in as this may look odd in apps that otherwise do not support dynamic type. |
The scope for this PR was to try only in Mapbox streets layers.
Right we need to implement this mechanism.
Agree, I need to research how this is handled on each platform. For this feature I discussed with @boundsj that we are not going to consider the customized font types. If you look at the image same weight applied to different font types have a different results thus implementing this as an opt-in. |
@fabian-guerra could you elaborate why "custom fonts" are an issue? We are using the scaling provided in our repository without any issue using a custom glyph atlas. |
@felixLam sure. Just in case when I say font type I refer to typeface. |
One option would be to allow developers to provide their own increments, like we do here: |
@felixLam thank you. I will take a look. |
Are accessible type settings uniform across these platforms? We should avoid a situation where we’re limited to a lowest common denominator of accessibility settings because of a desire to implement them all in core GL. |
What I'm proposing is to add this feature of scaling fonts to core, rather than implementing it for every platform. This would also allow us to add the scaling operation after function/expression evaluation for |
...sort of like |
Less work on the SDK’s part sounds great. 😄 What would the SDK tell core GL exactly? A universal scale factor would be simpler to implement, but the intent behind Dynamic Type is that labels would be scaled up based on their semantic roles (content size categories). For example, if we bake in Mapbox Streets source layer names, we could associate a country label with Would it be feasible to provide core GL with a table mapping source layer names to font sizes, rather than a universal scale factor? |
It would... however that leaves us with the same problem: When the user adds a custom layer, the SDK doesn't know about it and we don't scale that layer, unless we implement some sort of fallback scale value. Overall, I'm not a big fan of hardcoding logic that is geared to specific stylesheets. Adjusting individual labels at a different rate for maps seems like overengineering to me. |
We could provide an API delegate that takes care of this.
It may look but when you see an example it makes sense. I recommend watch this video and if you have an iPhone change the text size and see how Apple Maps reflects the changes. |
To clarify, I was referring to source layers like the ones documented here, not style layers. The SDK already has some logic specifically intended for the Mapbox Streets source as of #9582. |
|
||
if ([layer.textFontSize isKindOfClass:[MGLConstantStyleValue class]]) { | ||
NSNumber *textFontSize = [(MGLConstantStyleValue<NSNumber *> *)layer.textFontSize rawValue]; | ||
textFontSize = [NSNumber numberWithFloat:(textFontSize.floatValue * [self sizeForContentSizeCategory:preferredContentSize])]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could genericize the expression transformation in #11651 so that it could also be used to scale fonts according to the style layer’s content size category.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions. |
Fixes #7030
WIP