feat(theme): improved color contrast with color palette #28791
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Ionic's proposed color palette for dark theme is not WCAG AA compliant. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Updates the dark theme (`dark.css`) file with the proposed color palette for Level AA color contrast compliance. - Adds tests for verifying color contrast requirements for the following cases: - Base color on dark theme background (`#00000` on iOS an `#121212` on MD) - Contrast color on base color - Base color on base color with 0.08 opacity - Base color on base color with 0.12 opacity - Base color on base color with 0.16 opacity ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: Brandy Carney <brandy@ionic.io>
Issue number: None --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> There is no default body color. Note: This has not affected apps based on the starters since they contain `<meta name="color-scheme" content="light dark" />` and those provide a default text color of white or black based on the color scheme. However, if we wanted to change the default theme color from white or black to something a bit different, and if components aren't wrapped in an `<ion-content>`, it would be good to have this fallback. Also, currently, tests of some components need to be wrapped in an `<ion-content>` to get a default text color, since they don't have access to that `meta` tag. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - There is a default body color - Tests no longer need to be wrapped in an `<ion-content>` to have a default body color ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> This PR is targeting the color contrast feature branch.
## What is the current behavior? The current Ionic colors for the default (light) theme are: | Name | Base Value | Contrast Value | | ------------| -----------| ---------------| | `primary` | `#3880ff` | `#ffffff` | | `secondary` | `#3dc2ff` | `#ffffff` | | `tertiary` | `#5260ff` | `#ffffff` | | `success` | `#2dd36f` | `#ffffff` | | `warning` | `#ffc409` | `#000000` | | `danger` | `#eb445a` | `#ffffff` | | `light` | `#f4f5f8` | `#000000` | | `medium` | `#92949c` | `#ffffff` | | `dark` | `#222428` | `#ffffff` | These colors fail the WCAG AA requirements of a contrast ratio of at least `4.5:1`. ## What is the new behavior? The Ionic colors for the default (light) theme have been updated to the following in order to pass the WCAG AA contrast ratio: | Name | Base Value | Contrast Value | | ------------| -----------| ---------------| | `primary` | `#0054e9` | `#ffffff` | | `secondary` | `#0163aa` | `#ffffff` | | `tertiary` | `#6030ff` | `#ffffff` | | `success` | `#2dd55b` | `#000000` | | `warning` | `#ffc409` | `#000000` | | `danger` | `#c5000f` | `#ffffff` | | `light` | `#f6f8fc` | `#000000` | | `medium` | `#5f5f5f` | `#ffffff` | | `dark` | `#2f2f2f` | `#ffffff` | An e2e test has been added to check that all colors pass WCAG AA in the following scenarios: 1) The base color as the text color against the contrast color as the background color 2) The contrast color as the text color against the base color as the background color 3) The contrast color as the text color against the shade color as the background color 4) The contrast color as the text color against the tint color as the background color 5) The base color as the text color against the base color at 0.08 opacity as the background color 6) The base color as the text color against the base color at 0.12 opacity as the background color 7) The base color as the text color against the base color at 0.16 opacity as the background color A runnable test has been added that allows developers to switch between the available themes that are tested with the above scenarios ## Does this introduce a breaking change? - [x] Yes - [ ] No Breaking changes will be documented as part of the v8 release. --------- Co-authored-by: Sean Perkins <sean@ionic.io>
Issue number: N/A --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Axe tests are not run for light/dark theme ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Re-enabled the color-contrast test for range. This was disabled due to a (now fixed) Axe bug. - Moved the HTML templates into `setContent` so we can more easily test light/dark mode - Added tests for each component such that the color palette is used. Checked states were added for checkbox, radio, and toggle. Focused states were added for input, textarea, and select. The value of the range was set to 50% so we can see more of the selected track. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. -->
…tory (#28670) This updates our testing infrastructure for themes by doing the following: - Moves the `dark.css` theme file from `core/scripts/testing/themes/dark.css` → `core/src/themes/test/dark.css` - Updates this file to match the [recommended Dark Theme](https://ionicframework.com/docs/theming/dark-mode#ionic-dark-theme) with updated Ionic Colors - While updating to match the recommended theme we found a bug with the activated outline button when previewing the CSS variables test at `core/src/themes/test/css-variables/` — a [JIRA ticket](https://ionic-cloud.atlassian.net/browse/FW-5721) has been created to fix this on `main` - Moves the `default.css` theme file from `core/src/themes/test/css-variables/css/default.css` → `core/src/themes/test/default.css` - Updates this file to use the proper CSS variables with the new light theme colors on this branch, even though they are commented out - Updates paths for the tests to use the new path to the dark theme file - Moves the newly added code to set the `color` on the `body` into the core css file instead of being in the dark theme (84f5923)
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Uses of `ion-color()` in Button, Back Button, FAB, Menu Button, Item Option, and Segment do not have tests for color contrast. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Uses of `ion-color()` in Button, Back Button, FAB, Menu Button, Item Option, and Segment have tests for color contrast for both light and dark themes. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> - The action-sheet, alert, and toast components have tests for Axe violations, but the `color-contrast` rule is disabled. - The select-popover component does not test for Axe violations. - For all of the above plus loading, dark mode test for color contrast are not included. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Existing Axe tests pulled out into separate `configs` blocks that test both light and dark themes. Markup has also been converted to use `page.setContent` instead of `page.goto` to support automatic theme switching. - `color-contrast` rule re-enabled on action-sheet, alert, and toast. The rule was disabled due to contrast issues with the page content covered up by the backdrop, but with the new setup, the overlay is the only thing on the page. - For select-popover, the Axe test has been limited to only color contrast testing for now. The component had several Axe failures that will be addressed separately to keep scope down. (We didn't previously have any Axe testing on this component, which is why this wasn't caught.) - HTML file for the loading a11y test removed, since we're no longer using it. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. -->
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Maria noted that the way we set the main landmark is not correct and causes accessibility violations. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Instead of rendering a main element inside of ion-content, the content itself is the main landmark. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. -->
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> The dark theme is not exported for use in Ionic Framework apps ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - The dark theme is usable in Ionic Framework apps via any of the following: - The `ion-theme-dark` class (once the developer assigns this class to the `html` element in their app) via dark.class.css - The `prefers-color-scheme` media query via dark.system.css - Throughout the full app via dark.always.css ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Usage: ``` @import '~@ionic/core/css/themes/dark.always.css'; /* @import '~@ionic/core/css/themes/dark.class.css'; */ /* @import '~@ionic/core/css/themes/dark.system.css'; */ ``` replace `~@ionic/core` with the relevant package Dev build: `7.5.8-dev.11704294569.1abc989d` Note to reviewers: the design doc says to include these files in utils.bundle.scss. Doing this causes the dark theme to get applied by default because it includes the "always" theme in the bundled css used in Framework. Adding the imports in that file is not required for the files to get included in the built package; users can import them without them being included there. We can bikeshed the name of the "system" file.
chore: sync with feature-8.0
liamdebeasi
changed the title
liamdebeasi
requested review from
brandyscarney,
amandaejohnston,
sean-perkins and
a team
as code owners
sean-perkins
approved these changes
Jan 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number: Internal
What is the current behavior?
The team would like to ensure that Ionic Framework components that use an Ionic color (primary, secondary, etc) on top of a contrast color pass minimum contrast ratios as defined in the WCAG.
What is the new behavior?
Does this introduce a breaking change?
Other information