Skip to content

chore: revert OpenTransition prop change to fix undefined TransitionProps in builds #8671

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

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

Conversation

LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Aug 5, 2025

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:

Copy link
Member Author

Choose a reason for hiding this comment

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

@snowystinger any thoughts on the changes here? Was working on this with @devongovett and the below isn't great since setting children: ReactElement would mean we need to update the types for Overlay and a bunch of other things

Kinda feels like we just fix the build via props: any and call it since its internal only anyways

Copy link
Member

@snowystinger snowystinger Aug 6, 2025

Choose a reason for hiding this comment

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

I'm fine for it to be any since it's internal

Might be able to just revert the types for that file and turn off the eslint rule that was added as well for explicit modules if that's the issue

@rspbot
Copy link

rspbot commented Aug 5, 2025

LFDanLu added 2 commits August 7, 2025 09:41
deciding to do this for now since this is internal only and the types are problematic to implement
@LFDanLu LFDanLu changed the title chore: (WIP) define Transition props chore: revert OpenTransition prop change to fix undefined TransitionProps in builds Aug 7, 2025
@LFDanLu LFDanLu marked this pull request as ready for review August 7, 2025 16:44
@rspbot
Copy link

rspbot commented Aug 7, 2025

snowystinger
snowystinger previously approved these changes Aug 7, 2025
@@ -32,7 +32,8 @@ const OPEN_STATES = {
*/

export function OpenTransition(
props: TransitionProps
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
Copy link
Member

Choose a reason for hiding this comment

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

honestly, we should just turn off this eslint rule entirely. we decided we aren't going to do this
but I'm ok with that being follow up elsewhere

Copy link
Member Author

Choose a reason for hiding this comment

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

oh lol good point, pushed

@rspbot
Copy link

rspbot commented Aug 8, 2025

@rspbot
Copy link

rspbot commented Aug 8, 2025

## API Changes

react-aria-components

/react-aria-components:VisuallyHidden

-VisuallyHidden {
-  children?: ReactNode
-  className?: string | undefined
-  elementType?: string | JSXElementConstructor<any> = 'div'
-  id?: string | undefined
-  isFocusable?: boolean
-  role?: AriaRole | undefined
-  style?: CSSProperties | undefined
-  tabIndex?: number | undefined
-}

/react-aria-components:GridLayoutOptions

 GridLayoutOptions {
   dropIndicatorThickness?: number = 2
   maxColumns?: number = Infinity
-  maxHorizontalSpace?: number = Infinity
   maxItemSize?: Size = Infinity
   minItemSize?: Size = 200 x 200
   minSpace?: Size = 18 x 18
   preserveAspectRatio?: boolean = false

@internationalized/date

/@internationalized/date:setLocalTimeZone

-setLocalTimeZone {
-  timeZone: string
-  returnVal: undefined
-}

/@internationalized/date:resetLocalTimeZone

-resetLocalTimeZone {
-  returnVal: undefined
-}

@react-spectrum/overlays

/@react-spectrum/overlays:OpenTransition

 OpenTransition {
-  UNTYPED
+
 }

@react-stately/data

/@react-stately/data:ListData

 ListData <T> {
-  addKeysToSelection: (Selection) => void
   append: (Array<T>) => void
   filterText: string
   getItem: (Key) => T | undefined
   insert: (number, Array<T>) => void
   insertAfter: (Key, Array<T>) => void
   insertBefore: (Key, Array<T>) => void
   items: Array<T>
   move: (Key, number) => void
   moveAfter: (Key, Iterable<Key>) => void
   moveBefore: (Key, Iterable<Key>) => void
   prepend: (Array<T>) => void
   remove: (Array<Key>) => void
-  removeKeysFromSelection: (Selection) => void
   removeSelectedItems: () => void
   selectedKeys: Selection
   setFilterText: (string) => void
   setSelectedKeys: (Selection) => void
 }

/@react-stately/data:AsyncListData

 AsyncListData <T> {
-  addKeysToSelection: (Selection) => void
   append: (Array<T>) => void
   error?: Error
   filterText: string
   getItem: (Key) => T | undefined
   insert: (number, Array<T>) => void
   insertAfter: (Key, Array<T>) => void
   insertBefore: (Key, Array<T>) => void
   isLoading: boolean
   items: Array<T>
   loadMore: () => void
   loadingState: LoadingState
   move: (Key, number) => void
   moveAfter: (Key, Iterable<Key>) => void
   moveBefore: (Key, Iterable<Key>) => void
   prepend: (Array<T>) => void
   reload: () => void
   remove: (Array<Key>) => void
-  removeKeysFromSelection: (Selection) => void
   removeSelectedItems: () => void
   selectedKeys: Selection
   setFilterText: (string) => void
   setSelectedKeys: (Selection) => void
   sortDescriptor?: SortDescriptor
   update: (Key, T) => void
 }

@react-stately/layout

/@react-stately/layout:GridLayoutOptions

 GridLayoutOptions {
   dropIndicatorThickness?: number = 2
   maxColumns?: number = Infinity
-  maxHorizontalSpace?: number = Infinity
   maxItemSize?: Size = Infinity
   minItemSize?: Size = 200 x 200
   minSpace?: Size = 18 x 18
   preserveAspectRatio?: boolean = false

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants