Skip to content

Conversation

snowystinger
Copy link
Member

Reverts #5806

I've also made another commit to show how much threading through this requires. I still haven't done all the components. I think the color components, date/time pickers, every component with contextual help, and breadcrumbs, would all need to expose this prop too if we do it as it is currently setup.

I think this is a good time to consider doing a context based approach instead.

@rspbot
Copy link

rspbot commented Feb 26, 2024

@rspbot
Copy link

rspbot commented Feb 26, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
Item already in set
Section already in set
Section already in set
Section already in set

@react-spectrum/actionbar

Item

 SpectrumActionBarProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   children: ItemElement<T> | Array<ItemElement<T>> | ItemRenderer<T>
   disabledKeys?: Iterable<Key>
   isEmphasized?: boolean
   items?: Iterable<T>
   onClearSelection: () => void
   selectedItemCount: number | 'all'
 }
 

@react-spectrum/actiongroup

Item

 SpectrumActionGroupProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   buttonLabelBehavior?: 'show' | 'collapse' | 'hide' = 'show'
   children: ItemElement<T> | Array<ItemElement<T>> | ItemRenderer<T>
   density?: 'compact' | 'regular' = 'regular'
   disabledKeys?: Iterable<Key>
   isEmphasized?: boolean
   isJustified?: boolean
   isQuiet?: boolean
   items?: Iterable<T>
   onAction?: (Key) => void
   orientation?: Orientation = 'horizontal'
   overflowMode?: 'wrap' | 'collapse' = 'wrap'
   staticColor?: 'white' | 'black'
   summaryIcon?: ReactElement
 }
 

@react-spectrum/autocomplete

Section

 SpectrumSearchAutocompleteProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   align?: 'start' | 'end' = 'start'
   defaultInputValue?: string
   defaultItems?: Iterable<T>
   direction?: 'bottom' | 'top' = 'bottom'
   inputValue?: string
   isQuiet?: boolean
   items?: Iterable<T>
   loadingState?: LoadingState
   menuTrigger?: MenuTriggerAction = 'input'
   menuWidth?: DimensionValue
   onInputChange?: (string) => void
   onLoadMore?: () => void
   onOpenChange?: (boolean, MenuTriggerAction) => void
   onSubmit?: (string | null, Key | null) => void
   shouldFlip?: boolean = true
 }
 

@react-spectrum/combobox

Section

 SpectrumComboBoxProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   align?: 'start' | 'end' = 'end'
   allowsCustomValue?: boolean
   defaultInputValue?: string
   defaultItems?: Iterable<T>
   formValue?: 'text' | 'key' = 'text'
   inputValue?: string
   isQuiet?: boolean
   items?: Iterable<T>
   loadingState?: LoadingState
   menuTrigger?: MenuTriggerAction = 'input'
   menuWidth?: DimensionValue
   onInputChange?: (string) => void
   onOpenChange?: (boolean, MenuTriggerAction) => void
   onSelectionChange?: (Key | null) => any
   shouldFlip?: boolean = true
   shouldFocusWrap?: boolean
 }
 

@react-spectrum/dialog

DialogContainer

 SpectrumDialogContainerProps {
+  UNSTABLE_portalContainer?: Element = document.body
   children: ReactNode
   isDismissable?: boolean
   isKeyboardDismissDisabled?: boolean
   onDismiss: () => void
 }
 

useDialogContainer

 SpectrumDialogTriggerProps {
+  UNSTABLE_portalContainer?: Element = document.body
   children: [ReactElement, SpectrumDialogClose | ReactElement]
   hideArrow?: boolean
   isDismissable?: boolean
   isKeyboardDismissDisabled?: boolean
   targetRef?: RefObject<HTMLElement>
   type?: 'modal' | 'popover' | 'tray' | 'fullscreen' | 'fullscreenTakeover' = 'modal'
 }
 

@react-spectrum/menu

ContextualHelpTrigger

 SpectrumActionMenuProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   align?: Alignment = 'start'
   autoFocus?: boolean
   closeOnSelect?: boolean = true
   direction?: 'bottom' | 'top' | 'left' | 'right' | 'start' | 'end' = 'bottom'
   isQuiet?: boolean
   onAction?: (Key) => void
   shouldFlip?: boolean = true
   trigger?: MenuTriggerType = 'press'
 }
 

Section

 SpectrumMenuTriggerProps {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   align?: Alignment = 'start'
   children: Array<ReactElement>
   closeOnSelect?: boolean = true
   direction?: 'bottom' | 'top' | 'left' | 'right' | 'start' | 'end' = 'bottom'
   trigger?: MenuTriggerType = 'press'
 }
 

@react-spectrum/overlays

Modal

 Tray {
   children: ReactNode
+  container?: Element
   isFixedHeight?: boolean
   state: OverlayTriggerState
 }

@react-spectrum/picker

Section

 SpectrumPickerProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   align?: Alignment = 'start'
   autoComplete?: string
   autoFocus?: boolean
   defaultOpen?: boolean
   isOpen?: boolean
   isQuiet?: boolean
   menuWidth?: DimensionValue
   name?: string
   onOpenChange?: (boolean) => void
   shouldFlip?: boolean = true
 }
 

@react-spectrum/table

Column

 TableView<T extends {}> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
   dragAndDropHooks?: DragAndDropHooks['dragAndDropHooks']
   isQuiet?: boolean
   onAction?: (Key) => void
   onResizeEnd?: (Map<Key, ColumnSize>) => void
   onResizeStart?: (Map<Key, ColumnSize>) => void
   overflowMode?: 'wrap' | 'truncate' = 'truncate'
   renderEmptyState?: () => JSX.Element
 }
 

CellProps

 SpectrumTableProps<T> {
+  UNSTABLE_portalContainer?: HTMLElement = document.body
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
   dragAndDropHooks?: DragAndDropHooks['dragAndDropHooks']
   isQuiet?: boolean
   onAction?: (Key) => void
   onResizeEnd?: (Map<Key, ColumnSize>) => void
   onResizeStart?: (Map<Key, ColumnSize>) => void
   overflowMode?: 'wrap' | 'truncate' = 'truncate'
   renderEmptyState?: () => JSX.Element
 }
 

@react-spectrum/tooltip

TooltipTrigger

 SpectrumTooltipTriggerProps {
+  UNSTABLE_portalContainer?: Element = document.body
   children: [ReactElement, ReactElement]
   delay?: number = 1500
   isDisabled?: boolean
   offset?: number = 7
 }
 

Copy link
Member

Choose a reason for hiding this comment

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

I see this is the main offender for how many levels the portal container prop would need to go through, I think the idea of doing this via context that you suggested makes sense. Maybe we could specify the portal container override somewhere high level (Provider level?) and have component level overrides for specific cases where it may differ? I would assume most of the time there will only be one place a user would want to portal stuff to typically.

On the flip side, do we have to support this prop to such a degree across all the components or should we only add it when there is a large enough use case/ask for it?

Copy link
Member Author

@snowystinger snowystinger Feb 28, 2024

Choose a reason for hiding this comment

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

The demos I've seen have already made use of a few, menu/popover/tooltip. But they're also usually in fullscreen situations, which could easily be open to everything because it's basically just another app inside the fullscreen dialog situation.

@snowystinger
Copy link
Member Author

closing in favor of #6141

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.

3 participants