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
51.09k stars 13.51k forks source link

bug: Menu button is not using proper color due to :host-context on iOS Safari #19965

Closed seanaguinaga closed 4 years ago

seanaguinaga commented 4 years ago

Bug Report

Ionic version:

[x] 4.x

Current behavior:

View on iOS:

https://sean-preconceptiontest.tinytake.com/media/b68647?filename=1574354453447_iOS+Screenshot.png&sub_type=thumbnail_preview&type=attachment&width=370&height=798

Elements on iOS:

https://sean-preconceptiontest.tinytake.com/media/b68629?filename=1574354107689_iOS+Elements.png&sub_type=thumbnail_preview&type=attachment&width=1200&height=868

Expected behavior:

Color should changed based on background color on iOS

View: (on chrome) same as android 10 on Essential Phone

https://sean-preconceptiontest.tinytake.com/media/b68643?filename=1574354314403_desired+View.png&sub_type=thumbnail_preview&type=attachment&width=356&height=799

Elements on Chrome:

https://sean-preconceptiontest.tinytake.com/media/b6862d?filename=1574354172989_chrome+elements.png&sub_type=thumbnail_preview&type=attachment&width=1199&height=683

*Steps to reproduce:

export function WithMenu(props: IProps) {
  return (
    <IonHeader>
      <IonToolbar>
        <IonMenuButton slot="start" />
        <IonTitle>{props.pageTitle}</IonTitle>
      </IonToolbar>
    </IonHeader>
  );
}
  --ion-toolbar-border-color: #ffffff;
  --ion-toolbar-color: #ffffff;
  --ion-toolbar-color-activated: #ccb7fc;
  --ion-toolbar-color-unchecked: #165499;
  --ion-toolbar-color-checked: #1a8aac;

Ionic info:

Ionic:

   Ionic CLI       : 5.4.6 (C:\Users\sagui\AppData\Roaming\npm\node_modules\ionic)
   Ionic Framework : @ionic/react 4.11.5

Utility:

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

System:

   NodeJS : v12.13.0 (C:\Program Files\nodejs\node.exe)
   npm    : 6.12.0
   OS     : Windows 10
seanaguinaga commented 4 years ago

Just an inconsistency between platforms.

Shouldn’t :host-context be automatic?

Thank You Sean Aguiñaga

On Nov 21, 2019, at 09:52, Liam DeBeasi notifications@github.com wrote:

 Thanks for the issue. Are you reporting an issue with Ionic styles not working in Safari, or your own styles not working in Safari when applied using :host-context?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

liamdebeasi commented 4 years ago

:host-context is not supported in Safari, so we will have to implement a workaround in Ionic.

brandyscarney commented 4 years ago

Thanks for the issue! I actually noticed this while I was working on this segment PR and created a TODO comment for it: https://github.com/ionic-team/ionic/pull/19036/files#diff-7673edf3d4a944007672a209f5a97f54R128

host-context does not work in Safari with no plans to fix it: https://bugs.webkit.org/show_bug.cgi?id=160038

The solution will be to remove the host-context from the CSS and instead add it in the host element of menu-button like we are doing in segment: https://github.com/ionic-team/ionic/pull/19036/files#diff-b7598a09cd1196903cc9ebfeb1aff9b2R82-R83 https://github.com/ionic-team/ionic/pull/19036/files#diff-1c4aae98f93e4633c85e5c7209878fabR137-R139

seanaguinaga commented 4 years ago

That would do it!

Do you know of any manual tweaks I can apply?

Color=“white” doesn’t seem to work

Thank You Sean Aguiñaga

On Nov 21, 2019, at 09:58, Liam DeBeasi notifications@github.com wrote:

 :host-context is not supported in Safari, so we will have to implement a workaround in Ionic.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

brandyscarney commented 4 years ago

You should be able to just target the menu button directly:

ion-toolbar ion-menu-button {
  color: white;
}

https://codepen.io/brandyscarney/pen/abbMrXx

brandyscarney commented 4 years ago

ion-back-button will likely have this issue too since it was recently converted to shadow.

To further clarify - :host-context in Safari only breaks for components that are shadow components, it will work for scoped components.

seanaguinaga commented 4 years ago

Oh I see!

The back button seems to work.

Makes sense. I didn’t check the page elements for that!

Thank You Sean Aguiñaga

On Nov 21, 2019, at 10:11, Brandy Carney notifications@github.com wrote:

 ion-back-button will likely have this issue too since it was recently converted to shadow.

To further clarify - :host-context in Safari only breaks for components that are shadow components, it will work for scoped components.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

brandyscarney commented 4 years ago

Sorry back button was converted in Ionic 5 - you'll only see it if you're using the latest beta. 🙂

brandyscarney commented 4 years ago

Just to update this will be fixed by https://github.com/ionic-team/ionic/pull/19440

ionitron-bot[bot] commented 4 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.