Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Move onSurface.onSurfaceBackground to surface.surfaceSearchField #183

Merged
merged 3 commits into from
Mar 11, 2021

Conversation

AdamVig
Copy link
Contributor

@AdamVig AdamVig commented Mar 9, 2021

Commits

This fixes the non-unique `figmaName` of this variant, too, which
unblocks #178.
@AdamVig AdamVig self-assigned this Mar 9, 2021
@kaelig kaelig temporarily deployed to polaris-toke-fix-non-un-dzigq9 March 9, 2021 22:39 Inactive
Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

This makes sense to me but will need to be documented in CHANGELOG.md as it will likely break consumers using the current value.

Copy link
Contributor

@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.

The name change makes sense just be sure to add an entry to the changelog like Alex mentioned. Just under <!-- ## [Unreleased] --> at the top of the file

@kaelig kaelig temporarily deployed to polaris-toke-fix-non-un-dzigq9 March 10, 2021 16:16 Inactive
@AdamVig
Copy link
Contributor Author

AdamVig commented Mar 10, 2021

@alex-page @kyledurand Thanks for the reviews!

I added this to the changelog in the unreleased section. I made sure to mark this as a breaking change (cc @kaelig if that requires any special attention from you).

Copy link
Contributor

@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.

👍

Copy link
Contributor

@kaelig kaelig left a comment

Choose a reason for hiding this comment

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

There might be a few vault pages and Polaris pages that need updating — can you look into it?

@AdamVig
Copy link
Contributor Author

AdamVig commented Mar 10, 2021

@kaelig I found the following references of --p-on-surface-background and onSurfaceBackground:

  • polaris-styleguide
    • Three definitions in web/styleguide-demo/Demo/test/Demo.test.tsx, but it looks like they might be automatically generated
  • polaris-react
    • Three uses in src/components/TopBar/components/SearchField/SearchField.scss.
    • Documentation in documentation/Color system.md, but it looks like it is automatically updated when the tokens change.
    • Defined in src/components/README.md, does not seem to be automatically updated when tokens change.
  • No references that I could find in Vault.

I don't think I have time to cut a release of polaris-tokens and update those other repositories in the near future, but if that work is absolutely necessary in order to merge this, I can do it in a few weeks.

@alex-page
Copy link
Member

I can do the release this should be good to merge.

@alex-page alex-page merged commit fa8cd14 into main Mar 11, 2021
@alex-page alex-page deleted the fix-non-unique-figmaname branch March 11, 2021 01:12
@AdamVig
Copy link
Contributor Author

AdamVig commented Mar 11, 2021

Thanks, @alex-page!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants