ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.43k stars 13.53k forks source link

bug: item borders effectively invisible in md dark mode #29386

Closed aparajita closed 6 days ago

aparajita commented 1 week ago

Prerequisites

Ionic Framework Version

v8.x

Current Behavior

Maybe I'm missing something, but when using the Ionic dark mode palette in md mode, the separator lines for an ion-item within ion-list and ion-accordion become effectively invisible in dark mode.

In the case of an ion-item inside an ion-list, the border color is #222222, which is almost no contrast with the item backgound.

In the case of ion-accordion, the border color is set to --ion-color-shade, which is set to var(--ion-color-light-shade, #d7d8da) !important by the .ion-color-light class on the ion-item, which resolves to #1e2023, which is way too dark to be visible against the background.

I'm seeing this on the web and on an Android device.

Screenshot 2024-04-23 at 1 11 37 PM Screenshot 2024-04-23 at 1 11 57 PM

Expected Behavior

I would expect the borders to be lighter than the background with sufficient contrast to be visible to the naked eye, as they were in Ionic 7.

Steps to Reproduce

ion-accordion > ion-item reproduction: https://stackblitz.com/edit/motuzy-w4g1t1?file=src%2Fmain.ts ion-list > ion-item reproduction: https://stackblitz.com/edit/ryxtvq-ns4z3v?file=package.json

Code Reproduction URL

https://stackblitz.com/edit/ryxtvq-ns4z3v?file=package.json

Ionic Info

From stackblitz:

Ionic:

Ionic CLI : 7.2.0

Utility:

cordova-res : not installed globally native-run : not installed globally

System:

NodeJS : v18.18.0 npm : 10.2.3 OS : Linux 5.0

Additional Information

No response

liamdebeasi commented 1 week ago

We likely need to adjust the --ion-border-color value (or similar) for items in lists. I can reproduce the same behavior in Ionic 7: https://stackblitz.com/edit/motuzy-kmn4fz?file=package.json,src%2Fmain.ts,src%2Ftheme%2Fvariables.css,src%2Fenv.d.ts

The accordion has this problem because it has color="light" and we use the shade variant for the border.

aparajita commented 1 week ago

Thanks for taking a look.

liamdebeasi commented 1 week ago

Here's a dev build that should fix the contrast issue for Items without the color property: 8.0.1-dev.11713911458.1ad5f4cf

I'll need to take a closer look at how best to handle the case where color is set. For most colors in the palette, using the shade for the border is fine since we lighten the base color in dark mode. This is not the case for the light color in dark mode (and similarly, the dark color in light mode) since the base color actually gets darker.

liamdebeasi commented 6 days ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/29398, and a fix will be available in an upcoming release of Ionic Framework. This PR fixes the issue where the wrong border color was used on an item without the color prop in dark mode.

The other issue is a bit tricky to automatically solve for every use case. In some cases we need to use the tint for the border color, and in others we need to use the shade. This decision needs to be based on color contrast which we don't have a great way of automatically determining at runtime. Ideally we'd be able to use color-contrast which would choose the value that yields the higher contrast. However, the browser support for this is limited to Safari 15+ as of writing.

As browser support improves for color-contrast we'll be happy to consider adding support for this automatically. Here is a workaround you can implement in your application to resolve the issue for now:

@media (prefers-color-scheme: dark) {
  ion-item.ion-color-light::part(native) {
    border-color: var(--ion-color-tint);
  }
}