Skip to content

Conversation

ktabors
Copy link
Member

@ktabors ktabors commented Mar 23, 2022

Tech innovation project

Looked at existing components to determine which would benefit from stories with viewport width adjustments. These would be things like pickers and dialogs and usage of trays.

Components with new or adjusted stories:

  • Dialog
  • Menu
  • NumberField (We already have stories showing this as large scale. The new story just renders one in a narrow viewport.)
  • Picker
  • Provider (responsive stories)
  • Tabs

Some components like ComboBox, SearchField, SearchWithin, and SearchAutcomplete do not have isOpen to make useful stories in Chromatic. There are components like ActionMenu that don't have Chromatic stories so relevant viewport stories were not added.

✅ 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:

Test the new stories in this run of chromatic https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=228

🧢 Your Project:

RSP

@adobe-bot
Copy link

Build successful! 🎉

@adobe-bot
Copy link

Build successful! 🎉

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Just one question, otherwise looks good to me

'tray',
() => renderTriggerProps({type: 'tray'}), {
chromatic: {viewports: [320, 1200]},
chromaticProvider: {colorSchemes: ['light'], locales: ['ar-AE'], scales: ['large'], disableAnimations: true}
Copy link
Member

Choose a reason for hiding this comment

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

Just wanna make sure, but were these meant to be in ar-AE? It's fine it they are, just curious that they differ from the other chromatic stories in this file

Copy link
Member

Choose a reason for hiding this comment

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

Also any idea why the text seems to smaller than expected? Maybe something on Chromatic's end?
Never mind, its because of the locale

Copy link
Member Author

@ktabors ktabors Mar 23, 2022

Choose a reason for hiding this comment

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

I thought it would be useful to add some RTL (ar-AE) stories to dialog in Chromatic since we didn't have any.

Menu has a separate RTL file allowing all the stories to have the change if you'd rather see that approach. This change started as a focus on scale: large for the 320 viewport and I added RTL to it.

@adobe-bot
Copy link

Build successful! 🎉

@adobe-bot
Copy link

Build successful! 🎉

@ktabors ktabors merged commit 21cb067 into main Mar 30, 2022
@ktabors ktabors deleted the chromatic_viewports branch March 30, 2022 22:17
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