Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Black text color on dark theme #29219

Closed
3 tasks done
aeharding opened this issue Mar 26, 2024 · 4 comments
Closed
3 tasks done

bug: Black text color on dark theme #29219

aeharding opened this issue Mar 26, 2024 · 4 comments
Assignees
Labels
type: feature request a new feature, enhancement, or improvement

Comments

@aeharding
Copy link
Contributor

aeharding commented Mar 26, 2024

Prerequisites

Ionic Framework Version

v8.x (Beta)

Current Behavior

I noticed that buttons and badges have black text in dark mode of Ionic v8 beta.

This is contrary to native iOS apps in dark mode (buttons and badges) which have white text (see below).

Additionally, the colors seem a bit washed out compared to native iOS badge/buttons.

Expected Behavior

White text in dark mode for buttons and badges (primary/danger colors). More vibrant (iOS-like) primary/danger colors.

Steps to Reproduce

Here is black text in dark mode:

black text

Here is black text in danger badge:

image

Here is what I expect (iOS native app examples ignore red rectangle in first img):

Code Reproduction URL

https://stackblitz.com/edit/j1affd-djjrzh?file=src%2FApp.tsx

Ionic Info

N/A

Additional Information

Tangential: I think dark mode is broken in the ionic docs for v8. It only shows light mode.

@brandyscarney
Copy link
Member

Thanks for the issue! We have updated our default colors in v8 to meet the Web Content Accessibility Guidelines AA criteria for contrast ratio. You can read more about this in our blog post here: https://ionic.io/blog/announcing-the-ionic-8-beta

You can generate different colors by using our Color Generator in combination with a Dark Mode selector if desired.

@brandyscarney brandyscarney closed this as not planned Won't fix, can't repro, duplicate, stale Mar 26, 2024
@aeharding
Copy link
Contributor Author

@brandyscarney I made this issue at request from Discord, having black text in buttons and badges doesn't seem quite right to me, especially because I haven't seen any other app do that.

@brandyscarney brandyscarney reopened this Mar 26, 2024
@brandyscarney brandyscarney added the type: feature request a new feature, enhancement, or improvement label Mar 26, 2024
@ionitron-bot ionitron-bot bot removed the triage label Mar 26, 2024
@brandyscarney
Copy link
Member

Thanks for letting me know! I discussed this with the team and we will look into updating the dark palette colors while still passing WCAG AA compliance. I've marked this as a feature request as it is an enhancement.

liamdebeasi added a commit that referenced this issue Apr 15, 2024
Issue number: resolves #29219

---------

<!-- 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. -->

We got feedback that the current dark theme is a bit jarring when
compared with native. In particular, devs are expecting the contrast to
be white for many of these colors to match native iOS. Currently, text
inside of a primary button is black on dark mode, but devs expect it to
be white. Additionally, the dark mode colors appear to be too washed out
when compared with their light mode counterparts.

The team discussed this and we think we can find a way to make the
colors more in line with what devs expect while still allowing for AA
color contrast levels.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Adjusted the secondary and danger colors to be slightly more
vibrant/inline with what developers expect in mobile apps while still
meeting AA color contrast.

## 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

I intentionally did not change the contrast color, and I intentionally
did not significantly change the vibrancy of each color token. I did
some research into how the native platforms handle colors. I'll place
this information here, because there are some subtle differences between
web and native that make a difference.

### Material Design


[Source](https://m2.material.io/design/color/dark-theme.html#ui-application)

Material Design 2 calls for desaturated color tokens as well as flipping
contrast colors. For example, a vibrant purple background with light
text on light mode is a washed purple with dark text on dark mode. The
colors in Ionic <=7 did not follow these patterns. The tokens in Ionic 8
now follows these patterns, so what we have in `main` means we are
aligning closer with native MD than have in the past.


![image](https://github.com/ionic-team/ionic-framework/assets/2721089/b1067e7c-ee51-4170-91b1-27c0753fae72)

### iOS


[Source](https://developer.apple.com/design/human-interface-guidelines/accessibility#Color-and-effects)

iOS is a bit tricky. On the docs, Apple references the WCAG color
contrast formula (what Ionic follows). However, the table they present
below is slightly different:

| Text Size | Text Weight | Minimum Contrast Ratio |
| - | - | - |
| Up to 17 points | All | 4.5:1 |
| 18 points and larger | All | 3:1 |
| All | Bold | 3:1 |

The last row is the main difference. WCAG states that text that is bold
AND >=14pts (~18.66px) needs to meet a minimum contrast ratio of 3:1.
Apple's guidelines state that any text that is bold (regardless of size)
needs to meet a minimum contrast ratio of 3:1. In other words, **Apple
is using different guidelines to choose colors which is why colored dark
mode buttons on iOS typically use white text**.

However, Apple is inconsistent in implementing its own guidelines.
Consider the following red buttons in the Apple Music app.

| Screenshot | Meets Web guidelines | Meets Apple guidelines | Notes |
| - | - | - | - |
| <img
src="https://github.com/ionic-team/ionic-framework/assets/2721089/6bb0e0b3-c29e-4b96-9ad5-e45e59bf0415"
width="300" /> | ❌ | ❌ | Text is not bold which makes it not meet either
guidelines. |
| <img
src="https://github.com/ionic-team/ionic-framework/assets/2721089/8ce93b45-0b2b-4160-97a4-19852134684c"
width="300" /> | ❌ | ✅ | Text is bold which makes it pass the Apple
guidelines, but it's still too small to also pass the Web guidelines. |

### Ionic

One of the things I tried is adjusting the base color to work well with
white and black backgrounds. It's common to have a blue background with
white text (such as a button) AND to have a blue link on a black
background (such as an `a` element). This approach does not work well
for the shade/tint colors used for hover/focus states. These colors also
need to meet Web guidelines.

Consider the following example:

| Screenshot | Notes | Text/Link passes AA guidelines | Tint passes AA
guidelines |
| - | - | - | - |
| ![Screenshot 2024-04-15 at 3 28
47 PM](https://github.com/ionic-team/ionic-framework/assets/2721089/3d8c56a6-6d0c-4c93-bc13-9c93508f76bc)
| This uses the same primary color found in the light palette | ❌ | ✅ |
| ![Screenshot 2024-04-15 at 3 29
21 PM](https://github.com/ionic-team/ionic-framework/assets/2721089/beaf26c9-c30f-4a39-900e-74b0f2433280)
| This changes the base color such that the Text/Link passes with a
4.5:1 ratio | ✅ | ❌ |
| ![Screenshot 2024-04-15 at 3 34
14 PM](https://github.com/ionic-team/ionic-framework/assets/2721089/43dbd81c-1338-4fff-af30-efbe51fce08a)
| This changes the base color such that the Tint passes with a 4:5:1
ratio | ❌ | ✅ |

We also considered adjusting the `a` and `ion-text` colors to use the
tint color. We decided against that because it a) felt a little odd
given that tint is typically used for states (hover, focus, etc) and b)
we were concerned that making this change would impose future
restrictions on how this color palette can evolve.

### Conclusion

While Ionic's colors don't exactly match the iOS colors, the reality is
that we are a web-based tool, so we are going to be evaluated using
web-based tools/standards. As a result, it's recommended that we align
closer with the Web guidelines than Apple's guidelines.

## Testing

Testing:

Developers can look at the colors by opening
http://localhost:3333/src/themes/test/colors

In the video below, the first state is the dark theme in `main`. The
second state is the proposed changes.
| Visual Comparison |
| - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/241fb1b6-212b-4f03-83cd-9558b9a53761"></video>
|
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Apr 15, 2024

Thanks for the issue. This has been resolved via #29239, and a fix will be available in an upcoming release of Ionic Framework.

I wasn't able to make every requested change, but some of the colors are a bit more vibrant now. I posted a detailed explanation of some of our constraints as well as differences between iOS and web on the PR if you are interested in learning more: #29239

As for changing the contrast, the PR goes into more detail as to why we aren't changing that. However, you can definitely still use our base dark theme and then layer on your customizations on top of it if you'd like to keep the white contrast color for some of the buttons. Here's an example:

import '@ionic/react/css/palettes/dark.system.css';

@media (prefers-color-scheme: dark) {
  :root {
    --ion-color-primary-contrast: #ffffff;
    --ion-color-primary-contrast-rgb: 255, 255, 255;
    --ion-color-danger-contrast: #ffffff;
    --ion-color-danger-contrast-rgb: 255, 255, 255;
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request a new feature, enhancement, or improvement
Projects
None yet
Development

No branches or pull requests

3 participants