Skip to content

Conversation

@laurkim
Copy link
Contributor

@laurkim laurkim commented Oct 31, 2023

WHY are these changes introduced?

Fixes disabled button text color that was flagged on ContextualSaveBar during v12 bug hunt.

WHAT is this pull request doing?

Updates disabled button styles in ContextualSaveBar.


ContextualSaveBar — before
ContextualSaveBar — before


ContextualSaveBar — after
ContextualSaveBar — after

How to 🎩

Storybook

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';

import {Page, Frame, ContextualSaveBar} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
      <div style={{height: '250px'}}>
        <Frame
          logo={{
            width: 86,
            contextualSaveBarSource:
              'https://cdn.shopify.com/s/files/1/2376/3301/files/Shopify_Secondary_Inverted.png',
          }}
        >
          <ContextualSaveBar
            message="Unsaved changes"
            saveAction={{
              onAction: () => console.log('add form submit logic'),
              loading: false,
              disabled: true,
            }}
            discardAction={{
              onAction: () => console.log('add clear form logic'),
            }}
          />
        </Frame>
      </div>
    </Page>
  );
}

🎩 checklist

@laurkim laurkim added the Bug Something is broken and not working as intended in the system. label Oct 31, 2023
@laurkim laurkim self-assigned this Oct 31, 2023
@laurkim
Copy link
Contributor Author

laurkim commented Oct 31, 2023

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @laurkim! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

yarn add @shopify/polaris@0.0.0-snapshot-release-20231031134300

@laurkim laurkim force-pushed the lo/fix-disabled-button-styling branch from a62d812 to ca47072 Compare October 31, 2023 13:47
@laurkim laurkim marked this pull request as ready for review October 31, 2023 14:09
&.disabled {
color: var(--p-color-text-brand-on-bg-fill-disabled);
// stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list -- Set disabled text color with Button custom property
color: var(--pc-button-text);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should actually be --p-color-text-brand-on-bg-fill to set the color directly to a token value rather than an intermediary component custom property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this @aveline!

Had an offline conversation with @aveline and @sarahill and decided to update this pr to only change styling in the ContextualSaveBar component where the issue was first reported, as opposed to change button styling for all disabled Buttons 👍

@laurkim laurkim requested a review from sarahill October 31, 2023 15:40
@laurkim laurkim force-pushed the lo/fix-disabled-button-styling branch from ca47072 to badcb58 Compare October 31, 2023 16:08
@laurkim laurkim requested a review from aveline October 31, 2023 16:17
@laurkim laurkim merged commit a1cff35 into main Oct 31, 2023
@laurkim laurkim deleted the lo/fix-disabled-button-styling branch October 31, 2023 16:49
sophschneider pushed a commit that referenced this pull request Oct 31, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris@12.0.3

### Patch Changes

- [#11018](#11018)
[`38ed8a9ba`](38ed8a9)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Fixed
`TextField` blurring when interacting with the `Spinner` buttons


- [#11067](#11067)
[`a1cff3557`](a1cff35)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed disabled button
styling on `ContextualSaveBar` component


- [#11066](#11066)
[`9d5fedbce`](9d5fedb)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed styling on
`IndexTable` component header tooltip

## polaris.shopify.com@0.60.1

### Patch Changes

- Updated dependencies
\[[`38ed8a9ba`](38ed8a9),
[`a1cff3557`](a1cff35),
[`9d5fedbce`](9d5fedb)]:
    -   @shopify/polaris@12.0.3

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
### WHY are these changes introduced?

Fixes disabled button text color that was flagged on `ContextualSaveBar`
during v12 bug hunt.

### WHAT is this pull request doing?

Updates disabled button styles in `ContextualSaveBar`.
    <details>
      <summary>ContextualSaveBar — before</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/ed54d30a-2888-48d9-8f73-ec4caced2fea"
alt="ContextualSaveBar — before">
    </details>
    <details>
      <summary>ContextualSaveBar — after</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/66b3e9c3-0e2e-4f32-a763-2e5a81bd7dea"
alt="ContextualSaveBar — after">
    </details>

### How to 🎩


[Storybook](https://5d559397bae39100201eedc1-tcijtkhntq.chromatic.com/?path=/story/all-components-contextualsavebar--disabled)

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';

import {Page, Frame, ContextualSaveBar} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
      <div style={{height: '250px'}}>
        <Frame
          logo={{
            width: 86,
            contextualSaveBarSource:
              'https://cdn.shopify.com/s/files/1/2376/3301/files/Shopify_Secondary_Inverted.png',
          }}
        >
          <ContextualSaveBar
            message="Unsaved changes"
            saveAction={{
              onAction: () => console.log('add form submit logic'),
              loading: false,
              disabled: true,
            }}
            discardAction={{
              onAction: () => console.log('add clear form logic'),
            }}
          />
        </Frame>
      </div>
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
ascherkus pushed a commit to ascherkus/polaris that referenced this pull request Feb 19, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris@12.0.3

### Patch Changes

- [Shopify#11018](Shopify#11018)
[`38ed8a9ba`](Shopify@947fba6)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Fixed
`TextField` blurring when interacting with the `Spinner` buttons


- [Shopify#11067](Shopify#11067)
[`a1cff3557`](Shopify@71ff008)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed disabled button
styling on `ContextualSaveBar` component


- [Shopify#11066](Shopify#11066)
[`9d5fedbce`](Shopify@0679052)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed styling on
`IndexTable` component header tooltip

## polaris.shopify.com@0.60.1

### Patch Changes

- Updated dependencies
\[[`38ed8a9ba`](Shopify@947fba6),
[`a1cff3557`](Shopify@71ff008),
[`9d5fedbce`](Shopify@0679052)]:
    -   @shopify/polaris@12.0.3

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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 this pull request may close these issues.

3 participants