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.

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

fix: Chromatic/testing found bugs #7819

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

fix: Chromatic/testing found bugs #7819

wants to merge 15 commits into from

Conversation

snowystinger
Copy link
Member

@snowystinger snowystinger commented Feb 24, 2025

Closes
See bugs here https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=907

Summary:

  • Tree detached item gap 4px instead of 2px it should be according to tokens
  • TableView s2 empty is no longer vertically centering correctly

Other issues from testing

  • TreeView s2 empty state doesn't have a focus ring around an empty state with no interactive children
  • TreeView s2 is rendering all children always
  • RAC Toast docs display direction is reverse
  • Cannot visually tab to empty TreeView s2/v3 with no interactive children
  • TreeView v3 missing UNSAFE_className application
  • TreeView v3 missing height and overflow auto
  • TreeView v3 docs, some trees don't render child items

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

reidbarber
reidbarber previously approved these changes Feb 24, 2025
@rspbot
Copy link

rspbot commented Feb 24, 2025

@rspbot
Copy link

rspbot commented Feb 24, 2025

@rspbot
Copy link

rspbot commented Feb 24, 2025

@rspbot
Copy link

rspbot commented Feb 25, 2025

@rspbot
Copy link

rspbot commented Feb 25, 2025

@rspbot
Copy link

rspbot commented Feb 25, 2025

@snowystinger snowystinger changed the title fix: Chromatic found bugs fix: Chromatic/testing found bugs Feb 25, 2025
@rspbot
Copy link

rspbot commented Feb 25, 2025

@rspbot
Copy link

rspbot commented Feb 25, 2025

@rspbot
Copy link

rspbot commented Feb 25, 2025

@rspbot
Copy link

rspbot commented Feb 25, 2025

@@ -88,15 +88,6 @@ export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent
...rectStyles
};

if (layoutInfo.isSticky && !parent?.allowOverflow) {
Copy link
Member Author

Choose a reason for hiding this comment

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

haven't found any place that this code was affecting other than to make our S2 Tables empty and spinners not centered

@rspbot
Copy link

rspbot commented Feb 25, 2025

@rspbot
Copy link

rspbot commented Feb 26, 2025

@rspbot
Copy link

rspbot commented Feb 26, 2025

## API Changes

react-aria-components

/react-aria-components:Autocomplete

 Autocomplete {
   children: ReactNode
   defaultInputValue?: string
-  disableAutoFocusFirst?: boolean = false
   filter?: (string, string) => boolean
   inputValue?: string
   onInputChange?: (string) => void
   slot?: string | null

/react-aria-components:AutocompleteProps

 AutocompleteProps {
   children: ReactNode
   defaultInputValue?: string
-  disableAutoFocusFirst?: boolean = false
   filter?: (string, string) => boolean
   inputValue?: string
   onInputChange?: (string) => void
   slot?: string | null

@react-aria/autocomplete

/@react-aria/autocomplete:AriaAutocompleteProps

 AriaAutocompleteProps {
   children: ReactNode
   defaultInputValue?: string
-  disableAutoFocusFirst?: boolean = false
   filter?: (string, string) => boolean
   inputValue?: string
   onInputChange?: (string) => void
 }

/@react-aria/autocomplete:AriaAutocompleteOptions

 AriaAutocompleteOptions {
   collectionRef: RefObject<HTMLElement | null>
   defaultInputValue?: string
-  disableAutoFocusFirst?: boolean = false
   filter?: (string, string) => boolean
   inputRef: RefObject<HTMLInputElement | null>
   inputValue?: string
   onInputChange?: (string) => void

@react-spectrum/s2

/@react-spectrum/s2:TreeViewItem

-TreeViewItem <T extends {}> {
+TreeViewItem {
   aria-label?: string
-  childItems?: Iterable<{}>
   children: ReactNode
   download?: boolean | string
   hasChildItems?: boolean
   href?: Href
   id?: Key
   isDisabled?: boolean
   onAction?: () => void
   onHoverChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   ping?: string
   referrerPolicy?: HTMLAttributeReferrerPolicy
   rel?: string
   routerOptions?: RouterOptions
   target?: HTMLAttributeAnchorTarget
   textValue: string
   value?: T
 }

/@react-spectrum/s2:TreeViewItemProps

-TreeViewItemProps <T extends {} = {}> {
+TreeViewItemProps {
   aria-label?: string
-  childItems?: Iterable<{}>
   children: ReactNode
   download?: boolean | string
   hasChildItems?: boolean
   href?: Href
   id?: Key
   isDisabled?: boolean
   onAction?: () => void
   onHoverChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   ping?: string
   referrerPolicy?: HTMLAttributeReferrerPolicy
   rel?: string
   routerOptions?: RouterOptions
   target?: HTMLAttributeAnchorTarget
   textValue: string
   value?: T
 }

@react-spectrum/style-macro-s1

/@react-spectrum/style-macro-s1:focusRing

+focusRing {
+  returnVal: undefined
+}

@react-spectrum/tree

/@react-spectrum/tree:TreeViewItem

-TreeViewItem <T extends {}> {
+TreeViewItem {
   aria-label?: string
-  childItems?: Iterable<{}>
   children: ReactNode
   download?: boolean | string
   hasChildItems?: boolean
   href?: Href
   id?: Key
   isDisabled?: boolean
   onAction?: () => void
   ping?: string
   referrerPolicy?: HTMLAttributeReferrerPolicy
   rel?: string
   routerOptions?: RouterOptions
   target?: HTMLAttributeAnchorTarget
   textValue: string
 }

/@react-spectrum/tree:SpectrumTreeViewItemProps

-SpectrumTreeViewItemProps <T extends {} = {}> {
+SpectrumTreeViewItemProps {
   aria-label?: string
-  childItems?: Iterable<{}>
   children: ReactNode
   download?: boolean | string
   hasChildItems?: boolean
   href?: Href
   id?: Key
   isDisabled?: boolean
   onAction?: () => void
   ping?: string
   referrerPolicy?: HTMLAttributeReferrerPolicy
   rel?: string
   routerOptions?: RouterOptions
   target?: HTMLAttributeAnchorTarget
   textValue: string
 }

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.

4 participants