Skip to content

Conversation

@nicolleromero
Copy link
Contributor

@nicolleromero nicolleromero commented Jun 21, 2021

WHY are these changes introduced?

Fixes #4228

Fixes a bug in Safari that prevents the enabled state of the primary button from changing to white text when previously disabled.

WHAT is this pull request doing?

Adds a key to the text of the button component to force a re-paint when the primary button changes from disabled to enabled in Safari.

Disabled_button_demo.mov

Steps to Reproduce the Issue

  1. Open this Sandbox in Safari.
  2. Uncheck "Disabled".
  3. Button text will be gray instead of white.

OR

  1. Check out this branch (fix-disabled-button-text).
  2. Create a file at /playground/Playground.tsx.
  3. Add the following code to that file:
import React from 'react';

import {Button, Checkbox, Page} from '../src';

export function Playground() {
  const [disabled, setDisabled] = React.useState(true);

  return (
    <Page title="Playground">
      <Checkbox
        label="Disabled"
        checked={disabled}
        onChange={() => setDisabled(!disabled)}
      />
      <div>
        <Button disabled={disabled} primary>
          Broken in safari?
        </Button>
      </div>
    </Page>
  );
}

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2021

size-limit report

Path Size
cjs 142.03 KB (+0.01% 🔺)
esm 95.65 KB (+0.02% 🔺)
esnext 138.9 KB (+0.01% 🔺)
css 33.8 KB (0%)

@nicolleromero nicolleromero force-pushed the fix-disabled-button-text branch from 7da8174 to ee510ac Compare June 21, 2021 02:42
@nicolleromero nicolleromero changed the title [Button] Fix disabled button text in Safari [Button] Fix enabled button text in Safari Jun 21, 2021
@nicolleromero nicolleromero marked this pull request as ready for review June 21, 2021 03:13
Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing. Great find!

UNRELEASED.md Outdated

### Bug fixes

- Fix bug in Safari where button text is gray instead of white for enabled `Button` ([#4270](https://github.com/Shopify/polaris-react/pull/4270))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we elaborate a bit here? Maybe mention that it happens during a state change from disabled to enable and vice versa

@nicolleromero nicolleromero force-pushed the fix-disabled-button-text branch from ee510ac to e3ca429 Compare June 21, 2021 17:17
@nicolleromero nicolleromero merged commit eeaecae into main Jun 21, 2021
@nicolleromero nicolleromero deleted the fix-disabled-button-text branch June 21, 2021 17:39
@runmad
Copy link

runmad commented Jun 21, 2021

Thanks for fixing this!!! Nice find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Primary button has incorrect enabled state in Safari

3 participants