ionic-framework icon indicating copy to clipboard operation
ionic-framework copied to clipboard

feat: refresher should have global css variable to control background

Open infacto opened this issue 1 year ago • 3 comments

Prerequisites

Ionic Framework Version

  • [ ] v4.x
  • [ ] v5.x
  • [X] v6.x
  • [ ] Nightly

Current Behavior

Looks bad. 🤮 👎 refresher-current (GIF colors are not accurate. In real it looks a bit uglier.)

Expected Behavior

Look good. 😎 👍 refresher-expected

Steps to Reproduce

The refresher background and border should use either the same colors from item or card like --ion-card-background and --ion-border-color instead of --ion-color-step-250 and --ion-color-step-200.

The current colors looks ugly if you define all color-step variable for light and dark mode. There are also other issues in Ionic with color-step in light mode. I guess this is only tested for dark mode. But in this case, it looks weird in both themes.

My current workaround:

ion-refresher {
  --ion-color-step-250: var(--ion-card-background);
  --ion-color-step-200: var(--ion-border-color);
}

Code Reproduction URL

https://codepen.io/infacto/pen/QWmzBez

Ionic Info

@ionic/angular: 6.1.8 (ionic info not available, ionic CLI not in use for my monorepo.)

Additional Information

https://ionicframework.com/docs/api/refresher

https://github.com/ionic-team/ionic-framework/blob/436a8ce50847d26f8fb63ce64ea85be6f5c0bc8e/core/src/components/refresher/refresher.md.vars.scss#L12-L16

You can see e.g. the fallback #ffffff (white) of --ion-color-step-250 which is gray, if set.

infacto avatar Aug 18 '22 14:08 infacto

The refresher background should not be a step color here: https://github.com/ionic-team/ionic-framework/blob/436a8ce50847d26f8fb63ce64ea85be6f5c0bc8e/core/src/components/refresher/refresher.md.vars.scss#L16

When choosing step colors we should ensure that the step color aligns with the fallback. For example, the fallback is #ffffff but the default step color for 250 is #bfbfbf according to https://ionicframework.com/docs/theming/themes#stepped-colors. Components that do not align with step colors this way should get separate variables. We can add something like --ion-refresher-background to account for this.

A correct example would look something like the searcbhar. The clear icon is $text-color-step-400, which uses --ion-color-step-600: https://github.com/ionic-team/ionic-framework/blob/7315e0157b917d2e3586c64f8082026827812943/core/src/themes/ionic.theme.default.scss#L132. The fallback color is #666666 which aligns with the default value for --ion-color-step-600 on https://ionicframework.com/docs/theming/themes#stepped-color-generator

liamdebeasi avatar Aug 18 '22 14:08 liamdebeasi

A bit late, but I think this is more a bug than a feature, isn't it? Something what should be fixed.

infacto avatar Sep 21 '22 09:09 infacto

Fixing this requires exposing a new CSS Variable, which makes it a feature.

liamdebeasi avatar Sep 21 '22 13:09 liamdebeasi