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.

Sign up for GitHub

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

Jump to bottom

[DataTable] Consolidate conditional logic #10169

Merged
merged 1 commit into from
Aug 23, 2023
Merged

[DataTable] Consolidate conditional logic #10169

merged 1 commit into from
Aug 23, 2023

Conversation

sophschneider
Copy link
Contributor

@sophschneider sophschneider commented Aug 22, 2023 •
edited
Loading

WHY are these changes introduced?

Fixes #9930

WHAT is this pull request doing?

  • Consolidate se23 logic and styles for DataTable Component

How to 🎩

  • Compare production and this PR's chromatic storybook to make sure styles are the same

Production
This PR

@sophschneider
Copy link
Contributor Author

sophschneider commented Aug 22, 2023 •
edited
Loading

Pretty sure the 2 changed stories were both se23 specificity bugs and weren't intentional baselines. I will double check with @sarahill

Edit: Double checked and both changes are good to go 👍

Before After
Screenshot 2023-08-22 at 2 13 45 PM Screenshot 2023-08-22 at 2 14 01 PM
Before After
Screenshot 2023-08-22 at 2 14 46 PM Screenshot 2023-08-22 at 2 14 56 PM

@sophschneider sophschneider marked this pull request as ready for review August 22, 2023 18:16
laurkim
laurkim approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@laurkim laurkim left a comment

Choose a reason for hiding this comment

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

🙌 ✨

@sophschneider sophschneider merged commit 3a19bb9 into next Aug 23, 2023
13 checks passed
@sophschneider sophschneider deleted the sophie/data-table-cleanup branch August 23, 2023 16:12
sam-b-rose added a commit that referenced this pull request Aug 25, 2023
@sam-b-rose
Merge branch 'next' into update-next
8cf63b7 
* next: (87 commits)
  [ButtonGroup] Removed `segmented` boolean prop and replaced with `variant` (#9997)
  [LegacyTabs] Consolidate se23 styles and logic (#10231)
  [v12] Update changesets (#10232)
  [LegacyFilters] Consolidate se23 styles and logic (#10217)
  [Banner] Change status prop to tone (#10206)
  Replace icon highlight color in box stories (#10226)
  [TopBar] Consolidate se23 styles and logic (#10221)
  [Filters] consolidate se23 logic and styles (#10178)
  [Icon] Update props (#10220)
  [ButtonGroup] Update variant plain prop name (#10222)
  [Button] Remove connectedDisclosure (#10182)
  [LegacyCard] Consolidate se23 styles and logic (#10207)
  [Navigation] Consolidate se23 logic and styles (#10213)
  [Frame][ContextualSaveBar] Consolidate se23 styles and logic (#10196)
  [OptionList] consolidate se23 styles and logic  (#10177)
  [Page] Consolidate se23 styles and logic (#10187)
  [DataTable] Consolidate conditional logic (#10169)
  [Thumbnail] Consolidate conditional logic (#10171)
  [ResourceItem] Consolidate conditional logic (#10172)
  [FullscreenBar] Consolidate conditional logic (#10173)
  ...
sam-b-rose added a commit that referenced this pull request Aug 25, 2023
@sam-b-rose
Merge branch 'next' into update-next
6b4c634 
* next: (87 commits)
  [ButtonGroup] Removed `segmented` boolean prop and replaced with `variant` (#9997)
  [LegacyTabs] Consolidate se23 styles and logic (#10231)
  [v12] Update changesets (#10232)
  [LegacyFilters] Consolidate se23 styles and logic (#10217)
  [Banner] Change status prop to tone (#10206)
  Replace icon highlight color in box stories (#10226)
  [TopBar] Consolidate se23 styles and logic (#10221)
  [Filters] consolidate se23 logic and styles (#10178)
  [Icon] Update props (#10220)
  [ButtonGroup] Update variant plain prop name (#10222)
  [Button] Remove connectedDisclosure (#10182)
  [LegacyCard] Consolidate se23 styles and logic (#10207)
  [Navigation] Consolidate se23 logic and styles (#10213)
  [Frame][ContextualSaveBar] Consolidate se23 styles and logic (#10196)
  [OptionList] consolidate se23 styles and logic  (#10177)
  [Page] Consolidate se23 styles and logic (#10187)
  [DataTable] Consolidate conditional logic (#10169)
  [Thumbnail] Consolidate conditional logic (#10171)
  [ResourceItem] Consolidate conditional logic (#10172)
  [FullscreenBar] Consolidate conditional logic (#10173)
  ...
sophschneider added a commit that referenced this pull request Sep 19, 2023
@sophschneider
[DataTable] Consolidate conditional logic (#10169)
f9eee11 
### WHY are these changes introduced?

Fixes #9930

### WHAT is this pull request doing?

* Consolidate se23 logic and styles for `DataTable` Component

### How to 🎩
* Compare production and this PR's chromatic storybook to make sure
styles are the same


[Production](https://storybook.polaris.shopify.com/?path=/story/all-components-datatable--default&globals=polarisSummerEditions2023:true)
[This
PR](https://5d559397bae39100201eedc1-bhdulauigv.chromatic.com/?path=/story/all-components-datatable--default)
@sam-b-rose sam-b-rose mentioned this pull request Sep 19, 2023
Merged
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
@sophschneider
[DataTable] Consolidate conditional logic (Shopify#10169)
115e679 
### WHY are these changes introduced?

Fixes Shopify#9930

### WHAT is this pull request doing?

* Consolidate se23 logic and styles for `DataTable` Component

### How to 🎩
* Compare production and this PR's chromatic storybook to make sure
styles are the same


[Production](https://storybook.polaris.shopify.com/?path=/story/all-components-datatable--default&globals=polarisSummerEditions2023:true)
[This
PR](https://5d559397bae39100201eedc1-bhdulauigv.chromatic.com/?path=/story/all-components-datatable--default)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurkim laurkim laurkim approved these changes

@gwyneplaine gwyneplaine Awaiting requested review from gwyneplaine

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sophschneider @laurkim