fix: update secondary text color tokens#658
fix: update secondary text color tokens#658edschema wants to merge 6 commits intoopenedx:developfrom
Conversation
|
Thanks for the pull request, @edschema! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Description
Fix for this issue: openedx/wg-mobile#42 but extends beyond to fix general issues with textSecondary.
This PR changes the definition of the secondary color text tokens for accessibility purposes. The initial plan was to simply re-define textSecondary light and dark theme color definitions.
The current textSecondary colors are:
#97A5BBcontrast ratio: 2.5:1#B3B3B3contrast ratio: 2.1:1#79889Fcontrast ratio: 4.49:1#B3B3B3contrast ratio: 7.7:1Proposed colors:
#3D4964contrast ratio on white bg: 8.9:1#8E9BAEcontrast ratio on dark bg: 5.7:1Additionally, in reviewing the uses of textSecondary, I identified several other necessary changes.
Out of Scope (to be addressed later)
Additional Notes
In re-tokenizing icons, I also slightly shifted the color definition instead of keeping the old referenced "textSecondary" values; those values do not meet minimum contrast requirements. However, I used a slightly lighter shade than the new textSecondary colors. These colors should be evaluated as part of the future color design tokens work; the goal of this PR was to ensure accessibility.
I centralized the default course assignment colors on
CourseProgressGradingPolicybecause two colors were referencing textSecondary as a fallback color:Instead the fallback color (instead of referencing a text-related token) goes back to the default course colors.
fix: replace non-text textSecondary usages
Status: Implemented and committed.
Commit:
3b5eb17 fix: replace non-text textSecondary usagesChange Log
TabbarInactiveColorasset throughTheme.Colors.tabbarInactiveColor.textSecondaryLighttotabbarInactiveColor.textSecondary.opacity(0.5)tocourseProgressBG.textSecondaryto neutral card stroke tokens.textSecondarytocardViewStroke.textSecondarytoprogressSkip.textSecondarytoshadowColor.CourseProgressGradingPolicy.assignment_colorsresponses to the default assignment color palette.textSecondary.Notes
TextSecondaryorTextSecondaryLightasset values. It prepares for that follow-up by removing pure non-text dependencies on those text tokens.textSecondary/textSecondaryLightuses in the touched areas are text labels or mixed icon+text controls and are intentionally out of scope for this first pass.ThemeAssets.swiftis generated by SwiftGen and was not edited manually.TabbarInactiveColoralready existed in the asset catalog and UIKit theme surface; this change only adds the matching SwiftUI theme exposure.fix: update secondary text values and semantic retokens
Commits
86aa7d6 fix: add secondary color theme tokensb330a95 fix: retokenize placeholder and divider colorsf3cfa50 fix: retokenize empty state icon colorsf185ea5 fix: retokenize secondary content colorsd5a9d6e fix: retokenize inactive subtitle colorChange Log
TextSecondaryasset values:#3D4964#8E9BAETextSecondaryLightasset values to the same light/dark values while keeping the token separate.textSecondarytotextInputPlaceholderColor:TextEditorplaceholder inAppReviewViewDiscoveryViewtextSecondarytotextInputPlaceholderColor.#3D4964and dark#8E9BAE.textSecondary/textSecondaryLightto the new empty-state icon token:FullScreenErrorViewNoCoursesViewAppDownloadsViewCoursesToSyncViewTextTertiarytext color token:#5A6E8C#8E9BAEtextSecondarytoTextTertiarywhile keeping active subtitle lines ontextPrimary.SecondaryContentColortoken for small secondary content/chrome that is not text-only.DownloadCourseCellfromtextSecondarytoSecondaryContentColorso each icon and label stay visually equal.textSecondarytoSecondaryContentColor.textSecondarytotextPrimary.textSecondarytocardViewStroke.Expected Visual Impact
UI will look different
textSecondary/textSecondaryLighttext callsites will change color:#97A5BB->#3D4964#79889F->#8E9BAETextTertiary:#5A6E8C#8E9BAEtextPrimary.textSecondarytocardViewStroke:#97A5BB->#CCD3DF#79889F->#4E5A6FtextSecondarytotextPrimary, matching other text field labels.emptyStateIconColor, with light#3D4964and dark#8E9BAE. They will visually change from today's old secondary color values while no longer depending on text tokens.SecondaryContentColor, with the icon and label intentionally staying visually equal. IfSecondaryContentColoruses light#3D4964and dark#8E9BAE, these rows will visually change from today's old secondary color values.SecondaryContentColor, so it will visually change if that token uses the new secondary content values.No visual difference expected
textSecondarytotextInputPlaceholderColor.textSecondarytotextInputPlaceholderColor.textInputPlaceholderColorcurrently matches the old secondary values: light#97A5BB, dark#79889F, so these are semantic corrections that should preserve current appearance unless placeholder values change separately.