WRQ-11247: Added fontScale prop for large text mode scale values #3217
Conversation
Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3217 +/- ##
===========================================
- Coverage 82.36% 82.34% -0.03%
===========================================
Files 156 156
Lines 7209 7221 +12
Branches 1907 1908 +1
===========================================
+ Hits 5938 5946 +8
- Misses 999 1002 +3
- Partials 272 273 +1 ☔ View full report in Codecov by Sentry. |
Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
- Added fontScale to defaultConfig of ResolutionDecorator - Deleted fontScale to prevent it from being passed to Wrapped. Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
|
@mmyelyn, font scale does not work with http://SERVERIP:8080/?path=/story/about-a-tour-of-sampler--a-tour-of-sampler&globals=font+scale:1.6. Could you please check it? |
- Moved fontScale to defaultProps from defaultConfig - Modified it so that fontScale is applied on initial entry Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
…iable Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
This is because fontScale was not passed when updating the base font in init. In the process of fixing, i modified fontScale to be managed as an internal resolution variable. So, the fontScale parameter that was passed to the calculate function has been deleted. |
|
I think it is better to apply 4b93d18 commit to not change the original init function, but I need the merger's opinion. |
| /** | ||
| * Font Scale for large text mode. | ||
| * Use this value to set the scale of the font. | ||
| * This is the value that will be multiplied by pxPerRem, which is determined by resolution. |
There was a problem hiding this comment.
| /** | |
| * Font Scale for large text mode. | |
| * Use this value to set the scale of the font. | |
| * This is the value that will be multiplied by pxPerRem, which is determined by resolution. | |
| /** | |
| * Font scale value for the large text mode. | |
| * Use this value to set the scale of the font. | |
| * This is the value that will be multiplied by pxPerRem, which is determined by the resolution. |
There was a problem hiding this comment.
I think large text mode word may be confused with the original large text mode that is set in ThemeDecorator.
There was a problem hiding this comment.
I used it because the currently used term is large text mode. It may be confused with large text in ThemeDecorator, but since there is currently no fixed name, it would be a good idea to change it after the name is fixed. This is a draft and will not be merged.
There was a problem hiding this comment.
I agree. I think we need to set a proper name for this mode when merging it. I will approve it since this is a draft PR.
- Moved updateFontScale to constructor Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
regardless of the base font. Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
- FontScale must be updated before the scale function is called Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])
Checklist
Issue Resolved / Feature Added
There is a requirement for large text mode for a11y.
I have modified the app (including sampler) to review the screen when large text is applied.
Resolution
Modified to update the base font size when the fontScale prop changes.
Additional Considerations
Links
WRQ-11247
Comments
Enact-DCO-1.0-Signed-off-by: Hyelyn Kim ([email protected])