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

Primary button has incorrect enabled state in Safari #4228

Closed
runmad opened this issue May 25, 2021 · 6 comments · Fixed by #4270
Closed

Primary button has incorrect enabled state in Safari #4228

runmad opened this issue May 25, 2021 · 6 comments · Fixed by #4270
Labels
Bug Something is broken and not working as intended in the system.

Comments

@runmad
Copy link

runmad commented May 25, 2021

Issue summary

The enabled state of the primary button does not have white text on Safari. This works fine in Chrome. I did not test other browsers.

I could not replicate on polaris.shopify.com.

Expected behavior

This is what happens in Chrome:

Disabled:
chrome disabled state

Enabled:
chrome enabled state

Actual behavior

Disabled (correct):
save disabled state

Enabled:
save enabled state

Steps to reproduce the problem

  1. Log into Shopify admin using Safari
  2. Go to a product details page
  3. Make an edit.
  4. Notice that the save button changes state but the text is hard to read.

Reduced test case

Again, I am not able to reproduce on polaris.shopify.com or in the sandbox.

The best way to get your bug fixed is to provide a reduced test case. This CodeSandbox template is a great starting point.

Specifications

  • Are you using the React components? (Y/N): Y
  • Polaris version number: This is in production, live on /admin rights now.
  • Browser: Safari
  • Device: Mac
  • Operating System: macOS
@runmad runmad added the Bug Something is broken and not working as intended in the system. label May 25, 2021
@runmad
Copy link
Author

runmad commented May 26, 2021

Debugged a little further and this only appears to be an issue for buttons inside the ContexualSaveBar and I was able to replicate in the example app and saw that the color of the text is not set correctly, but didn't have a chance to dive into what's causing the issue.

@IlanaB
Copy link

IlanaB commented Jun 9, 2021

I'm seeing a similar issue in Safari with the primary button on our refreshed login page. This only happens in that browser and if I even start to change any styles via developers console it fixes itself. Other ways this seems to self resolve is through any re-render or resizing the screen past a breakpoint (triggers rerender).

@kyledurand
Copy link
Contributor

Thanks for that example @IlanaB that helped me debug a bit.

I think the issue is stemming from a state change and safari not rendering the correct color value after everything has loaded. I've made a little sandbox to reproduce: Sandbox

That said, in the second example I added everything is fine in Safari. The styles are inline which makes things simpler but it leads me to believe that the issue lies somewhere in Polaris.

I'll try to do some digging when I have time but if anyone has any ideas I'm all 👂

@runmad
Copy link
Author

runmad commented Jun 10, 2021

Good test... Oddly I can make it look fine if I am switching to another tab in Safari. Doesn't happen every time, but when I switched back it's resolved:

Screen Shot 2021-06-10 at 3 16 43 PM

@runmad
Copy link
Author

runmad commented Jun 15, 2021

Also seeing this for the Delete button when in the Delete apps modal:
Screen Shot 2021-06-15 at 2 30 33 PM

@kyledurand
Copy link
Contributor

Thanks @runmad I had a feeling that would happen as well since critical is using the same button-filled mixin. I can reproduce locally but running out of ideas on how to fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken and not working as intended in the system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants