Skip to content

Enable strict mode in storybook on a per story basis#3333

Merged
LFDanLu merged 11 commits into
mainfrom
strict_mode_per_story
Dec 20, 2022
Merged

Enable strict mode in storybook on a per story basis#3333
LFDanLu merged 11 commits into
mainfrom
strict_mode_per_story

Conversation

@LFDanLu
Copy link
Copy Markdown
Member

@LFDanLu LFDanLu commented Jul 22, 2022

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

Comment thread .storybook/strictmode.js Outdated
Comment on lines +18 to +23
<Checkbox
marginTop={10}
isSelected={selected}
onChange={() => setSelected(selected => !selected)}>
Toggle strict mode
</Checkbox>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

TODO: still trying to figure out if it is possible to have this be a option in the controls panel instead

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't put it there, but you could put it up in the toolbar across the top of the stories, where we put the locale/scale/theme

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

urgh, I was hoping that https://storybook.js.org/docs/react/essentials/controls#parameters was alluding to being able to set a control param globally but putting it in the toolbar sounds good as well

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you probably could through a decorator but I think it'd be a lot more work than it's worth, plus you'd have to apply it to every component, there's a number of them with no controls
I think toolbar is the better solution

@adobe-bot
Copy link
Copy Markdown

@adobe-bot
Copy link
Copy Markdown

Comment thread .storybook/custom-addons/strictmode/index.js
@LFDanLu LFDanLu marked this pull request as ready for review December 14, 2022 00:31
@LFDanLu LFDanLu changed the title (WIP) Enable strict mode in storybook on a per story basis Enable strict mode in storybook on a per story basis Dec 14, 2022
let updateStrict = (val) => {
setStrict(val);
};
channel.on('strict/updated', updateStrict);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I would've like to used the storybook globals but for some reason it stopped working

Copy link
Copy Markdown
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Well, the storybook seems to be running just fine. This is somewhat problematic for me though, since I can't seem to do anything that gives a StrictMode error. Have you been able to see any?

@LFDanLu
Copy link
Copy Markdown
Member Author

LFDanLu commented Dec 14, 2022

A good candidate is ComboBox (or anything that uses a overlay I believe).

@snowystinger
Copy link
Copy Markdown
Member

A good candidate is ComboBox (or anything that uses a overlay I believe).

o, I figured it out, can't test it in storybook built by CI, that's production.... haha, have to do it locally

Copy link
Copy Markdown
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM.

@LFDanLu LFDanLu mentioned this pull request Dec 20, 2022
5 tasks
Copy link
Copy Markdown
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Great, I can see NumberField not working when strict mode is on and it works when strict mode is off.

@LFDanLu LFDanLu merged commit 7bf553f into main Dec 20, 2022
@LFDanLu LFDanLu deleted the strict_mode_per_story branch December 20, 2022 22:23
LFDanLu added a commit that referenced this pull request Dec 21, 2022
LFDanLu added a commit that referenced this pull request Dec 21, 2022
@LFDanLu LFDanLu restored the strict_mode_per_story branch December 21, 2022 17:36
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.

4 participants