[Storybook] Fix stories containing Modal components #10492

mrcthms · 2023-09-15T13:35:32Z

WHY are these changes introduced?

Due to a recent update to the Dialog component within the Modal, it now uses the useFrame hook. Due to this, we need to ensure that anywhere it's used, we have a parent <Frame> component anywhere. This is not an issue for production, where the <Frame> is automatically included by the <AppProvider> or for tests where mountWithApp includes the <Frame>, but I noticed some Storybook stories breaking. So we wrap all stories in a <FrameContext.Provider> component now.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Go to Storybook generated by Chromatic
Go to the Banner -> In a Modal story
It should work fine
Go to the IndexFilters stories
Launch the Create new View modal
They should work fine
Go to the IndexTable stories that contain the filtering element
Launch the Create new View modal
They should work fine

🎩 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

sophschneider · 2023-09-15T14:19:45Z

Should we add Frame as a decorator in https://github.com/Shopify/polaris/blob/main/polaris-react/.storybook/preview.js so we don't have to wrap the stories in it every time we want to use a modal?

kyledurand · 2023-09-15T14:23:22Z

This is not an issue for production, where the is automatically included by the

I don't think this is true. I'm wondering if the previous modal pr was technically a breaking change. Can you link to that PR?

mrcthms · 2023-09-15T14:26:21Z

Should we add Frame as a decorator in https://github.com/Shopify/polaris/blob/main/polaris-react/.storybook/preview.js so we don't have to wrap the stories in it every time we want to use a modal?

Yeah, that looks like a much nicer solution, thanks for the tip!

mrcthms · 2023-09-15T14:27:14Z

@kyledurand Here's the PR, and the change specifically where we need frame context.

We use the useFrame hook in various other components, Toast, Navigation, TopBar, Loading, so it didn't feel breaking to include it in the Dialog. Where could you see this being an issue?

kyledurand · 2023-09-15T14:30:52Z

Where could you see this being an issue?

Should be pretty minor. I can just see some app dev who created a simple app with a modal but didn't need the frame for some reason. It's why I said it's technically a breaking change. Hopefully no one complains, if they do it's an easy fix for them.

mrcthms · 2023-09-15T14:34:53Z

@kyledurand got it, thanks for the heads up. And yeah, thankfully easy to fix for those potential edge cases 🙏

mrcthms · 2023-09-15T15:33:16Z

.changeset/six-donkeys-nail.md

+'@shopify/polaris': patch
+---
+
+Updates Storybook stories containing Modals to be wrapped in a Frame component


Suggested change

Updates Storybook stories containing Modals to be wrapped in a Frame component

Updates Storybook stories containing Modals to be wrapped in a Frame component

Updates Storybook stories to be wrapped with an empty FrameContext.Provider

kyledurand

👍

kyledurand · 2023-09-15T14:17:01Z

.changeset/six-donkeys-nail.md

+'@shopify/polaris': patch
+---
+
+Updates Storybook stories containing Modals to be wrapped in a Frame component


Suggested change

Updates Storybook stories containing Modals to be wrapped in a Frame component

Updated Storybook stories containing Modals to be wrapped in a Frame component

Always get caught out by present tense/past tense in these descriptions 🫠

.changeset/six-donkeys-nail.md

Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com>

@aaronccasanova

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@11.20.0 ### Minor Changes - [#10477](#10477) [`790a001cd`](790a001) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated semantic `color` tokens - [#10478](#10478) [`8be227e0c`](8be227e) Thanks [@MaxCloutier](https://github.com/MaxCloutier)! - Added `allowFiltering` prop on `ActionList`, and `filterActions` prop on Page Header - [#9445](#9445) [`7be9c243a`](7be9c24) Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added support for subheaders and selection of a range of `IndexTable.Rows` -- See the [With subheaders](https://polaris.shopify.com/components/tables/index-table) example on polaris.shopify.com for how to properly configure - `IndexTable.Row` - Added support for setting the `indeterminate` value on the `selected` prop - Added the `selectionRange` prop to specify a range of other consecutive, related rows selected when the row is selected - Added the `rowType` prop to indicate the relationship or role of the row's contents (defaults to `data`, `subheader` renders the row to look and behave like the table header row) Added support for setting accessibility attributes on `IndexTable.Cell` - `IndexTable.Cell` - Added the `as` prop to support rendering the cell as a `th` element if it is serving as a subheading cell - Added support for the `headers` attribute to manually associate all headers when the cell is described by more than its column heading - Added support for the `colSpan` attribute to specify the number of the columns that the cell element should extend to - Added support for the `scope` attribute to indicate whether the `th` is a header for a column, row, or group of columns or rows - [#10490](#10490) [`863f15ff2`](863f15f) Thanks [@mrcthms](https://github.com/mrcthms)! - Add new `IndexFiltersManager` for allowing disabling of Page Header actions when in Filtering or EditingColumns mode - [#10566](#10566) [`9fed74317`](9fed743) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed a bug in `Filters` where changes to the `appliedFilters` prop were not being handled ### Patch Changes - [#10404](#10404) [`5acfcec04`](5acfcec) Thanks [@jesstelford](https://github.com/jesstelford)! - Scoped CSS variables for non-responsive props on `Tooltip`, `RangeSlider`, `ProgressBar`, and `HorizontalStack`. - [#10582](#10582) [`3efbc1b4e`](3efbc1b) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed the focus states of actions within the Page Header component - [#10492](#10492) [`d5ff72dec`](d5ff72d) Thanks [@mrcthms](https://github.com/mrcthms)! - Updated Storybook stories to be wrapped with an empty FrameContext.Provider - Updated dependencies \[[`fe1aac1b5`](fe1aac1), [`790a001cd`](790a001), [`63cf3ad24`](63cf3ad), [`120e96eae`](120e96e)]: - @shopify/polaris-tokens@7.10.0 ## @shopify/polaris-tokens@7.10.0 ### Minor Changes - [#10465](#10465) [`fe1aac1b5`](fe1aac1) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated private primitive `colors` - [#10477](#10477) [`790a001cd`](790a001) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated semantic `color` tokens - [#10600](#10600) [`63cf3ad24`](63cf3ad) Thanks [@lgriffee](https://github.com/lgriffee)! - Added public primitive and semantic `shadow` token scales ### Patch Changes - [#10485](#10485) [`120e96eae`](120e96e) Thanks [@lgriffee](https://github.com/lgriffee)! - Updated public primitive `base` and `light-uplift` theme scales ## @shopify/polaris-migrator@0.22.5 ### Patch Changes - [#10575](#10575) [`ea6b54284`](ea6b542) Thanks [@aveline](https://github.com/aveline)! - Handled `buttonFrom` and `buttonsFrom` functions in `Button` migration - Updated dependencies \[[`fe1aac1b5`](fe1aac1), [`790a001cd`](790a001), [`63cf3ad24`](63cf3ad), [`120e96eae`](120e96e)]: - @shopify/polaris-tokens@7.10.0 - @shopify/stylelint-polaris@14.0.5 ## @shopify/stylelint-polaris@14.0.5 ### Patch Changes - Updated dependencies \[[`fe1aac1b5`](fe1aac1), [`790a001cd`](790a001), [`63cf3ad24`](63cf3ad), [`120e96eae`](120e96e)]: - @shopify/polaris-tokens@7.10.0 ## polaris.shopify.com@0.57.8 ### Patch Changes - [#10605](#10605) [`9748b0838`](9748b08) Thanks [@laurkim](https://github.com/laurkim)! - Updated logic for rendering `color` custom property previews in `TokenList` - [#10573](#10573) [`da09e0b8c`](da09e0b) Thanks [@kyledurand](https://github.com/kyledurand)! - Updated copy url to change browser url - Updated dependencies \[[`fe1aac1b5`](fe1aac1), [`5acfcec04`](5acfcec), [`790a001cd`](790a001), [`8be227e0c`](8be227e), [`63cf3ad24`](63cf3ad), [`7be9c243a`](7be9c24), [`863f15ff2`](863f15f), [`3efbc1b4e`](3efbc1b), [`d5ff72dec`](d5ff72d), [`120e96eae`](120e96e), [`9fed74317`](9fed743)]: - @shopify/polaris-tokens@7.10.0 - @shopify/polaris@11.20.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

### WHY are these changes introduced? Due to a recent update to the Dialog component within the Modal, it now uses the `useFrame` hook. Due to this, we need to ensure that anywhere it's used, we have a parent `<Frame>` component anywhere. This is not an issue for production, where the `<Frame>` is automatically included by the `<AppProvider>` or for tests where `mountWithApp` includes the `<Frame>`, but I noticed some Storybook stories breaking. So we wrap all stories in a `<FrameContext.Provider>` component now. ### How to 🎩 🖥 [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) - Go to Storybook generated by Chromatic - Go to the Banner -> In a Modal story - It should work fine - Go to the IndexFilters stories - Launch the Create new View modal - They should work fine - Go to the IndexTable stories that contain the filtering element - Launch the Create new View modal - They should work fine ### 🎩 checklist - [x] 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) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [x] Updated the component's `README.md` with documentation changes - [x] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide --------- Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com>

mrcthms added 2 commits September 15, 2023 16:24

chore: add changeset

4f4c774

chore: wrap stories in FrameContext

3c51e1b

mrcthms force-pushed the mrcthms-modal-story-fix branch from f75fb86 to 3c51e1b Compare September 15, 2023 15:30

mrcthms commented Sep 15, 2023

View reviewed changes

chore: update changeset

b6c900f

kyledurand approved these changes Sep 15, 2023

View reviewed changes

Update .changeset/six-donkeys-nail.md

77da59d

Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com>

mrcthms merged commit d5ff72d into main Sep 15, 2023
11 checks passed

mrcthms deleted the mrcthms-modal-story-fix branch September 15, 2023 16:09

shopify-github-actions-access bot mentioned this pull request Sep 15, 2023

Version Packages #10495

Merged

jesstelford mentioned this pull request Sep 18, 2023

[Sandbox] Mock Frame functions in the Sandbox to avoid hard failures #10507

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Storybook] Fix stories containing Modal components #10492

[Storybook] Fix stories containing Modal components #10492

mrcthms commented Sep 15, 2023 •

edited

Loading

sophschneider commented Sep 15, 2023

kyledurand commented Sep 15, 2023

mrcthms commented Sep 15, 2023

mrcthms commented Sep 15, 2023 •

edited

Loading

kyledurand commented Sep 15, 2023

mrcthms commented Sep 15, 2023

mrcthms Sep 15, 2023

kyledurand left a comment

kyledurand Sep 15, 2023

mrcthms Sep 15, 2023

	Updates Storybook stories containing Modals to be wrapped in a Frame component
	Updates Storybook stories containing Modals to be wrapped in a Frame component
	Updates Storybook stories to be wrapped with an empty FrameContext.Provider

	Updates Storybook stories containing Modals to be wrapped in a Frame component
	Updated Storybook stories containing Modals to be wrapped in a Frame component

[Storybook] Fix stories containing Modal components #10492

[Storybook] Fix stories containing Modal components #10492

Conversation

mrcthms commented Sep 15, 2023 • edited Loading

WHY are these changes introduced?

How to 🎩

🎩 checklist

sophschneider commented Sep 15, 2023

kyledurand commented Sep 15, 2023

mrcthms commented Sep 15, 2023

mrcthms commented Sep 15, 2023 • edited Loading

kyledurand commented Sep 15, 2023

mrcthms commented Sep 15, 2023

mrcthms Sep 15, 2023

Choose a reason for hiding this comment

kyledurand left a comment

Choose a reason for hiding this comment

kyledurand Sep 15, 2023

Choose a reason for hiding this comment

mrcthms Sep 15, 2023

Choose a reason for hiding this comment

mrcthms commented Sep 15, 2023 •

edited

Loading

mrcthms commented Sep 15, 2023 •

edited

Loading