Conversation
0d8402d to
c9618cd
Compare
c9618cd to
d4cbe9a
Compare
d4cbe9a to
1b32ad7
Compare
1b32ad7 to
67d7fa7
Compare
67d7fa7 to
856a1f4
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new exported HvDropdownPanel component (refactoring the prior internal BaseDropdownPanel) and migrates dropdown-like components and docs examples to use it, centralizing Popper + ClickAwayListener behavior and aligning placement-driven styling.
Changes:
- Refactor
BaseDropdownPanelinto the exportedHvDropdownPanelusing MUIPopper+ClickAwayListenerand shared Popper modifiers. - Update
HvBaseDropdown,HvSelect,HvDropdown, andHvDropDownMenuto use the new panel and placement propagation approach. - Refresh styles/examples and adjust CI Storybook sharding.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/themes/pentaho.ts | Adds theme slot overrides for HvDropdownPanel and keeps legacy popper styling for existing dropdowns. |
| packages/core/src/Select/Select.tsx | Replaces direct Popper usage with HvDropdownPanel for the Select panel. |
| packages/core/src/Select/Select.styles.ts | Removes panel/popup styling now expected to be provided by HvDropdownPanel/HvPanel. |
| packages/core/src/ListContainer/ListContainer.styles.tsx | Adds padding/margin workaround around overflowClipMargin to improve Safari behavior. |
| packages/core/src/FilterGroup/FilterGroup.test.tsx | Adjusts a test interaction/assertion to match updated focus behavior. |
| packages/core/src/DropDownMenu/DropDownMenu.tsx | Tracks computed Popper placement and forwards it for placement-based styling. |
| packages/core/src/DropdownButton/DropdownButton.tsx | Removes the placement prop and relies on callers to provide data-popper-placement when needed. |
| packages/core/src/Dropdown/List/List.tsx | Removes BaseDropdown context dependency and derives max size from passed popper styles. |
| packages/core/src/Dropdown/List/List.styles.tsx | Removes padding/margin now handled elsewhere (ListContainer styles). |
| packages/core/src/Dropdown/Dropdown.tsx | Captures popper element styles during container creation and passes them to the list. |
| packages/core/src/BaseDropdown/utils.ts | Introduces a shared usePopperModifiers hook for width/size modifiers and placement tracking. |
| packages/core/src/BaseDropdown/index.ts | Re-exports the new panel from the BaseDropdown barrel. |
| packages/core/src/BaseDropdown/context.ts | Removes the old Popper context (no longer needed with HvDropdownPanel). |
| packages/core/src/BaseDropdown/BaseDropdownPanel.tsx | Implements the new exported HvDropdownPanel component. |
| packages/core/src/BaseDropdown/BaseDropdown.tsx | Refactors HvBaseDropdown to use HvDropdownPanel and updated container creation signature. |
| packages/core/src/BaseDropdown/BaseDropdown.styles.tsx | Removes panel/container styling now owned by HvDropdownPanel. |
| packages/core/src/AppSwitcher/AppSwitcher.styles.tsx | Removes padding/margin now handled by updated ListContainer behavior. |
| apps/docs/src/app/examples/login/LoginShort.tsx | Migrates example popper usage to HvDropdownPanel. |
| apps/docs/src/app/examples/login/LoginFull.tsx | Migrates example popper usage to HvDropdownPanel (but includes a leftover debug log). |
| apps/docs/src/app/examples/inputs/TagsInputDropdown.tsx | Migrates example popper usage to HvDropdownPanel. |
| apps/docs/src/app/examples/inputs/DropdownPrefix.tsx | Updates Select usage/styling and enables variableWidth. |
| .github/workflows/tests.yml | Increases Storybook test sharding from 2 to 3. |
| export interface HvDropdownPanelProps | ||
| extends Omit<PopperProps, "children">, | ||
| Pick<HvBaseDropdownProps, "disablePortal" | "onClickOutside" | "children"> { | ||
| variableWidth?: boolean; | ||
| classes?: ExtractNames<typeof useClasses>; | ||
| containerId?: string; | ||
| onContainerKeyDown: (event: any) => void; | ||
| onToggle?: (event: any) => void; | ||
| onFirstUpdate?: OptionsGeneric<any>["onFirstUpdate"]; | ||
| onClickAway: ClickAwayListenerProps["onClickAway"]; | ||
| } |
There was a problem hiding this comment.
HvDropdownPanelProps extends HvBaseDropdownProps with onClickOutside, but the implementation doesn't use it, and instead requires a separate onClickAway prop. This forces consumers (e.g. HvSelect) to pass no-op handlers and adds unnecessary global listeners. Consider either: (1) make onClickAway optional and only wrap with ClickAwayListener when provided, or (2) accept onClickOutside and wire it internally to ClickAwayListener so callers don't need both concepts.
856a1f4 to
77ceed5
Compare
77ceed5 to
03eaa4d
Compare
03eaa4d to
540bdda
Compare
8c1884c to
c39d1e6
Compare
c39d1e6 to
5ee6a1f
Compare
chore: refactor code chore: rename & misc
5ee6a1f to
70d2c78
Compare
BaseDropdownPaneltoHvDropdownPanelPopperandClickAwayListenerfeaturesHvDropdownPanelHvSelect