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

[3.0.0-beta] Scoped styles not working on iOS using attribute strategy #8165

Closed
1 task
fflaten opened this issue Aug 20, 2023 · 10 comments
Closed
1 task

[3.0.0-beta] Scoped styles not working on iOS using attribute strategy #8165

fflaten opened this issue Aug 20, 2023 · 10 comments
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: styling Related to styles (scope)

Comments

@fflaten
Copy link
Contributor

fflaten commented Aug 20, 2023

What version of astro are you using?

3.0.0-beta.4

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

pnpm

What operating system are you using?

iOS

What browser are you using?

Safari

Describe the Bug

Scoped styling are not applying properly on iOS when using the default strategy attribute in 3.0. Everything works in Edge on Windows, but not on iOS. Tested using both iOS 16 and 17 beta:
broken

Reverting to class or where strategy works as expected.

Seeing issues with multiple components, ex the theme toggle below:

---

---

<button id="themeToggle" type="button" aria-label="Switch between light and dark theme">
  <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
    <path class="sun" fill-rule="evenodd" d="M12 17.5a5.5 5.5 0 1 0 0-11 5.5 5.5 0 0 0 0 11zm0 1.5a7 7 0 1 0 0-14 7 7 0 0 0 0 14zm12-7a.8.8 0 0 1-.8.8h-2.4a.8.8 0 0 1 0-1.6h2.4a.8.8 0 0 1 .8.8zM4 12a.8.8 0 0 1-.8.8H.8a.8.8 0 0 1 0-1.6h2.5a.8.8 0 0 1 .8.8zm16.5-8.5a.8.8 0 0 1 0 1l-1.8 1.8a.8.8 0 0 1-1-1l1.7-1.8a.8.8 0 0 1 1 0zM6.3 17.7a.8.8 0 0 1 0 1l-1.7 1.8a.8.8 0 1 1-1-1l1.7-1.8a.8.8 0 0 1 1 0zM12 0a.8.8 0 0 1 .8.8v2.5a.8.8 0 0 1-1.6 0V.8A.8.8 0 0 1 12 0zm0 20a.8.8 0 0 1 .8.8v2.4a.8.8 0 0 1-1.6 0v-2.4a.8.8 0 0 1 .8-.8zM3.5 3.5a.8.8 0 0 1 1 0l1.8 1.8a.8.8 0 1 1-1 1L3.5 4.6a.8.8 0 0 1 0-1zm14.2 14.2a.8.8 0 0 1 1 0l1.8 1.7a.8.8 0 0 1-1 1l-1.8-1.7a.8.8 0 0 1 0-1z"/>
    <path class="moon" fill-rule="evenodd" d="M16.5 6A10.5 10.5 0 0 1 4.7 16.4 8.5 8.5 0 1 0 16.4 4.7l.1 1.3zm-1.7-2a9 9 0 0 1 .2 2 9 9 0 0 1-11 8.8 9.4 9.4 0 0 1-.8-.3c-.4 0-.8.3-.7.7a10 10 0 0 0 .3.8 10 10 0 0 0 9.2 6 10 10 0 0 0 4-19.2 9.7 9.7 0 0 0-.9-.3c-.3-.1-.7.3-.6.7a9 9 0 0 1 .3.8z"/>
  </svg>
</button>

<style lang="scss">
  button {
    background-color: transparent;
    border: 0;
    padding: 0;
    cursor: pointer;
  }
  svg {
    display: block;
    width: 32px;
    height: 32px;

    @media (hover: hover) {
      path {
        transition: all 0.2s ease-in;

        &:hover {
          transition: all 0.2s ease-out;
        }
      }
    }

    .sun { fill: black; }
    .moon { fill: transparent; }
  }

  :global([data-theme='dark']) svg {
    .sun { fill: transparent; }
    .moon { fill: white; }
  }
</style>

Related #7893

What's the expected result?

Scoped styles works on iOS.
working

Link to Minimal Reproducible Example

See comment

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Aug 20, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Aug 20, 2023

Thanks for creating this issue!

Unfortunately, it's not possible to work on this without a reproduction. You don't have to use stackblitz, codesandbox or a repo would be helpful too.

On a different note, it seems to be a browser compat issue, not an Astro one. Have you tried debugging this in Safari desktop?

@lilnasy lilnasy added needs repro Issue needs a reproduction and removed needs triage Issue needs to be triaged labels Aug 20, 2023
@github-actions
Copy link
Contributor

Hello @fflaten. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@fflaten
Copy link
Contributor Author

fflaten commented Aug 20, 2023

On a different note, it seems to be a browser compat issue, not an Astro one.

True, but the breaking change to attribute default strategy is an Astro issue. The default should be the best and most compatible value - and Safari is too big to ignore. 🙂

Have you tried debugging this in Safari desktop?

I don't have access to a Mac unfortunately. I assumed the provided component would work as a repro in a new blank project, but will verify it.

@fflaten
Copy link
Contributor Author

fflaten commented Aug 20, 2023

Looks like repro is a lot easier:

  1. create-astro with default template
  2. Upgrade to astro@beta
  3. Test on Safari iOS. Astro-logo is unstyled: huge and above heading

Even though workaround exist, this is P4 issue imo.

@lilnasy
Copy link
Contributor

lilnasy commented Aug 20, 2023

Thanks for this, and it really is a browser compat issue that should make us reconsider the default.

Astro default template rendered: https://lilnasy.github.io/astro-8165
The logo svg with astro-a class doesn't receive its expected styles because the selector doesnt match it.

document.querySelector(".astro-a[data-astro-cid-J7PV25F6]")
On Chrome (matches successfully) image
On Firefox (fails) image
On Safari (fails) image

@lilnasy lilnasy added feat: styling Related to styles (scope) - P4: important Violate documented behavior or significantly impacts performance (priority) and removed needs repro Issue needs a reproduction labels Aug 20, 2023
@fflaten
Copy link
Contributor Author

fflaten commented Aug 20, 2023

Thanks. How did you test Safari on Windows?
Since Firefox failed too I had a chance to troubleshoot.

The selector hash is uppercased while the attribute is lowercase. Lowercasing the selector works on Firefox. Could you verify on Safari?

@lilnasy
Copy link
Contributor

lilnasy commented Aug 20, 2023

The selector hash is uppercased while the attribute is lowercase.

Thanks for looking into it! That's reassuring, it means we can fix it on our side without changing the default.

How did you test Safari on Windows?

Playwright builds. They build and release on their CDN every few days. The latest build number is 1888, https://playwright.azureedge.net/builds/webkit/1888/webkit-win64.zip.

@ematipico
Copy link
Member

We talked about lower casing the hash in the compiler. This issue confirms that we should do it

@lilnasy
Copy link
Contributor

lilnasy commented Aug 20, 2023

@lilnasy
Copy link
Contributor

lilnasy commented Aug 22, 2023

Fixed in #8180, released as [email protected].

@lilnasy lilnasy closed this as completed Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: styling Related to styles (scope)
Projects
None yet
Development

No branches or pull requests

3 participants