Skip to content

Conversation

yihuiliao
Copy link
Member

@yihuiliao yihuiliao commented Sep 26, 2024

Adds chromatic stories for all our S2 components and removes the S2 prefix so that our S2 stories don't fall under that heading.

We still need to improve our S2 chromatic story coverage since I mostly just copy and pasted from our docs storybook but figured we can get this in first and then work on that.

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

@@ -6,7 +6,6 @@ module.exports = {
},
stories: [
'../packages/**/chromatic/**/*.stories.@(js|jsx|ts|tsx)',
'../packages/@react-spectrum/s2/stories/*.stories.@(js|jsx|mjs|ts|tsx)',
Copy link
Member Author

Choose a reason for hiding this comment

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

removed this since we don't need to run chromatic on our s2 docs storybook anymore

@yihuiliao
Copy link
Member Author

question about this line:

'../packages/@react-spectrum/s2/stories/*.stories.@(js|jsx|mjs|ts|tsx)'

now that we've gotten rid of the S2 prefix, all the S1 and S2 story's get jumbled together so it's hard to tell which is which. Do we want to create a separate chromatic-fc for S2 or run it on our S2 chromatic storybook or leave it like this for now?

// }
layout: 'centered',
chromatic: {
disableSnapshot: true
Copy link
Member Author

Choose a reason for hiding this comment

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

do we still need this disableSnapshot if we don't plan to run chromatic on the normal docs stories?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should need it anymore

@rspbot
Copy link

rspbot commented Sep 26, 2024

@snowystinger
Copy link
Member

question about this line:

'../packages/@react-spectrum/s2/stories/*.stories.@(js|jsx|mjs|ts|tsx)'

now that we've gotten rid of the S2 prefix, all the S1 and S2 story's get jumbled together so it's hard to tell which is which. Do we want to create a separate chromatic-fc for S2 or run it on our S2 chromatic storybook or leave it like this for now?

I think we should change it to run against our chromatic stories instead?

@rspbot
Copy link

rspbot commented Sep 26, 2024

@devongovett
Copy link
Member

Can we separate the docs and these other examples for style macro? I'd like to just have all the docs at the top without needing to open a sub-folder.

image

Well should probably just be a real component anyway. It's actually quite a bit of styles for everyone to paste. And since these examples don't show the code, you'd need to find the story source in order to copy it so not sure how useful it is to show here.

@devongovett
Copy link
Member

Oh, never mind, these stories are excluded from the docs anyway.

@rspbot
Copy link

rspbot commented Sep 26, 2024

@reidbarber
Copy link
Member

reidbarber commented Sep 26, 2024

@devongovett the Well story was just added for us to verify the codemod styles used to replace it.

@rspbot
Copy link

rspbot commented Sep 26, 2024

@rspbot
Copy link

rspbot commented Sep 26, 2024

## API Changes

@react-spectrum/s2

/@react-spectrum/s2:Accordion

 Accordion {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   allowsMultipleExpanded?: boolean
   children: React.ReactNode
   defaultExpandedKeys?: Iterable<Key>
-  density?: 'compact' | 'regular' | 'spacious' = "regular"
+  density?: 'compact' | 'regular' | 'spacious' = 'regular'
   expandedKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
   isQuiet?: boolean
   onExpandedChange?: (Set<Key>) => any
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
 }

/@react-spectrum/s2:ColorSwatch

 ColorSwatch {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   color?: string | Color
   colorName?: string
   id?: string
-  rounding?: 'default' | 'none' | 'full' = "default"
-  size?: 'XS' | 'S' | 'M' | 'L' = "M"
+  rounding?: 'default' | 'none' | 'full' = 'default'
+  size?: 'XS' | 'S' | 'M' | 'L' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
 }

/@react-spectrum/s2:ColorSwatchPicker

 ColorSwatchPicker {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   children: ReactNode
   defaultValue?: string | Color
-  density?: 'compact' | 'regular' | 'spacious' = "regular"
+  density?: 'compact' | 'regular' | 'spacious' = 'regular'
   onChange?: (Color) => void
-  rounding?: 'none' | 'default' | 'full' = "none"
-  size?: 'XS' | 'S' | 'M' | 'L' = "M"
+  rounding?: 'none' | 'default' | 'full' = 'none'
+  size?: 'XS' | 'S' | 'M' | 'L' = 'M'
   slot?: string | null
   styles?: StylesProp
   value?: string | Color
 }

/@react-spectrum/s2:ComboBox

 ComboBox <T extends {}> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
-  align?: 'start' | 'end' = "start"
+  align?: 'start' | 'end' = 'start'
   allowsCustomValue?: boolean
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   children: ReactNode | ({}) => ReactNode
   contextualHelp?: ReactNode
   defaultInputValue?: string
   defaultItems?: Iterable<T>
   defaultSelectedKey?: Key
   description?: ReactNode
-  direction?: 'bottom' | 'top' = "bottom"
+  direction?: 'bottom' | 'top' = 'bottom'
   disabledKeys?: Iterable<Key>
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   formValue?: 'text' | 'key' = 'key'
   id?: string
   inputValue?: string
   isDisabled?: boolean
   isInvalid?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   items?: Iterable<T>
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   menuTrigger?: MenuTriggerAction = 'input'
   menuWidth?: number
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<HTMLInputElement>) => void
   onFocus?: (FocusEvent<HTMLInputElement>) => void
   onFocusChange?: (boolean) => void
   onInputChange?: (string) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean, MenuTriggerAction) => void
   onSelectionChange?: (Key | null) => void
   selectedKey?: Key | null
   shouldFlip?: boolean = true
   shouldFocusWrap?: boolean
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
   validate?: (ComboBoxValidationValue) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'

/@react-spectrum/s2:Disclosure

 Disclosure {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   children: ReactNode
   defaultExpanded?: boolean
-  density?: 'compact' | 'regular' | 'spacious' = "regular"
+  density?: 'compact' | 'regular' | 'spacious' = 'regular'
   id?: Key
   isDisabled?: boolean
   isExpanded?: boolean
   isQuiet?: boolean
   onExpandedChange?: (boolean) => void
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
 }

/@react-spectrum/s2:NumberField

 NumberField {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   contextualHelp?: ReactNode
   decrementAriaLabel?: string
   defaultValue?: number
   description?: ReactNode
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   formatOptions?: Intl.NumberFormatOptions
   hideStepper?: boolean = false
   id?: string
   incrementAriaLabel?: string
   isDisabled?: boolean
   isInvalid?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   isWheelDisabled?: boolean
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   maxValue?: number
   minValue?: number
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBeforeInput?: FormEventHandler<HTMLInputElement>
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (T) => void
   onCompositionEnd?: CompositionEventHandler<HTMLInputElement>
   onCompositionStart?: CompositionEventHandler<HTMLInputElement>
   onCompositionUpdate?: CompositionEventHandler<HTMLInputElement>
   onCopy?: ClipboardEventHandler<HTMLInputElement>
   onCut?: ClipboardEventHandler<HTMLInputElement>
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onInput?: FormEventHandler<HTMLInputElement>
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPaste?: ClipboardEventHandler<HTMLInputElement>
   onSelect?: ReactEventHandler<HTMLInputElement>
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   step?: number
   styles?: StylesProp
   validate?: (number) => ValidationError | boolean | null | undefined
   value?: number
 }

/@react-spectrum/s2:Picker

 Picker <T extends {}> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
-  align?: 'start' | 'end' = "start"
+  align?: 'start' | 'end' = 'start'
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoComplete?: string
   autoFocus?: boolean
   children: ReactNode | ({}) => ReactNode
   contextualHelp?: ReactNode
   defaultOpen?: boolean
   defaultSelectedKey?: Key
   description?: ReactNode
-  direction?: 'bottom' | 'top' = "bottom"
+  direction?: 'bottom' | 'top' = 'bottom'
   disabledKeys?: Iterable<Key>
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   excludeFromTabOrder?: boolean
   id?: string
   isInvalid?: boolean
   isOpen?: boolean
   isRequired?: boolean
   items?: Iterable<T>
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   menuWidth?: number
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean) => void
   onSelectionChange?: (Key) => void
   placeholder?: string
   selectedKey?: Key | null
   shouldFlip?: boolean = true
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
   validate?: (Key) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'
 }

/@react-spectrum/s2:Tabs

 Tabs {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children?: ReactNode
   defaultSelectedKey?: Key
-  density?: 'compact' | 'regular' = "regular"
+  density?: 'compact' | 'regular' = 'regular'
   disabledKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
   keyboardActivation?: 'automatic' | 'manual' = 'automatic'
   orientation?: Orientation = 'horizontal'
   selectedKey?: Key | null
   slot?: string | null
   styles?: StylesPropWithHeight
 }

/@react-spectrum/s2:AccordionProps

 AccordionProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   allowsMultipleExpanded?: boolean
   children: React.ReactNode
   defaultExpandedKeys?: Iterable<Key>
-  density?: 'compact' | 'regular' | 'spacious' = "regular"
+  density?: 'compact' | 'regular' | 'spacious' = 'regular'
   expandedKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
   isQuiet?: boolean
   onExpandedChange?: (Set<Key>) => any
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
 }

/@react-spectrum/s2:ColorSwatchProps

 ColorSwatchProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   color?: string | Color
   colorName?: string
   id?: string
-  rounding?: 'default' | 'none' | 'full' = "default"
-  size?: 'XS' | 'S' | 'M' | 'L' = "M"
+  rounding?: 'default' | 'none' | 'full' = 'default'
+  size?: 'XS' | 'S' | 'M' | 'L' = 'M'
   slot?: string | null
   styles?: StylesPropWithHeight
 }

/@react-spectrum/s2:ColorSwatchPickerProps

 ColorSwatchPickerProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   children: ReactNode
   defaultValue?: string | Color
-  density?: 'compact' | 'regular' | 'spacious' = "regular"
+  density?: 'compact' | 'regular' | 'spacious' = 'regular'
   onChange?: (Color) => void
-  rounding?: 'none' | 'default' | 'full' = "none"
-  size?: 'XS' | 'S' | 'M' | 'L' = "M"
+  rounding?: 'none' | 'default' | 'full' = 'none'
+  size?: 'XS' | 'S' | 'M' | 'L' = 'M'
   slot?: string | null
   styles?: StylesProp
   value?: string | Color
 }

/@react-spectrum/s2:ComboBoxProps

 ComboBoxProps <T extends {}> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
-  align?: 'start' | 'end' = "start"
+  align?: 'start' | 'end' = 'start'
   allowsCustomValue?: boolean
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   children: ReactNode | ({}) => ReactNode
   contextualHelp?: ReactNode
   defaultInputValue?: string
   defaultItems?: Iterable<T>
   defaultSelectedKey?: Key
   description?: ReactNode
-  direction?: 'bottom' | 'top' = "bottom"
+  direction?: 'bottom' | 'top' = 'bottom'
   disabledKeys?: Iterable<Key>
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   formValue?: 'text' | 'key' = 'key'
   id?: string
   inputValue?: string
   isDisabled?: boolean
   isInvalid?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   items?: Iterable<T>
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   menuTrigger?: MenuTriggerAction = 'input'
   menuWidth?: number
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<HTMLInputElement>) => void
   onFocus?: (FocusEvent<HTMLInputElement>) => void
   onFocusChange?: (boolean) => void
   onInputChange?: (string) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean, MenuTriggerAction) => void
   onSelectionChange?: (Key | null) => void
   selectedKey?: Key | null
   shouldFlip?: boolean = true
   shouldFocusWrap?: boolean
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
   validate?: (ComboBoxValidationValue) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'

/@react-spectrum/s2:DisclosureProps

 DisclosureProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   children: ReactNode
   defaultExpanded?: boolean
-  density?: 'compact' | 'regular' | 'spacious' = "regular"
+  density?: 'compact' | 'regular' | 'spacious' = 'regular'
   id?: Key
   isDisabled?: boolean
   isExpanded?: boolean
   isQuiet?: boolean
   onExpandedChange?: (boolean) => void
-  size?: 'S' | 'M' | 'L' | 'XL' = "M"
+  size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
 }

/@react-spectrum/s2:PickerProps

 PickerProps <T extends {}> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
-  align?: 'start' | 'end' = "start"
+  align?: 'start' | 'end' = 'start'
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoComplete?: string
   autoFocus?: boolean
   children: ReactNode | ({}) => ReactNode
   contextualHelp?: ReactNode
   defaultOpen?: boolean
   defaultSelectedKey?: Key
   description?: ReactNode
-  direction?: 'bottom' | 'top' = "bottom"
+  direction?: 'bottom' | 'top' = 'bottom'
   disabledKeys?: Iterable<Key>
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   excludeFromTabOrder?: boolean
   id?: string
   isInvalid?: boolean
   isOpen?: boolean
   isRequired?: boolean
   items?: Iterable<T>
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   menuWidth?: number
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean) => void
   onSelectionChange?: (Key) => void
   placeholder?: string
   selectedKey?: Key | null
   shouldFlip?: boolean = true
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
   validate?: (Key) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'
 }

/@react-spectrum/s2:TabsProps

 TabsProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children?: ReactNode
   defaultSelectedKey?: Key
-  density?: 'compact' | 'regular' = "regular"
+  density?: 'compact' | 'regular' = 'regular'
   disabledKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
   keyboardActivation?: 'automatic' | 'manual' = 'automatic'
   orientation?: Orientation = 'horizontal'
   selectedKey?: Key | null
   slot?: string | null
   styles?: StylesPropWithHeight
 }

@devongovett devongovett added this pull request to the merge queue Sep 26, 2024
Merged via the queue into main with commit c353e15 Sep 26, 2024
29 checks passed
@devongovett devongovett deleted the s2-chromatic-story-coverage branch September 26, 2024 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants