Skip to content

Use internal context system to apply toolbar variant to toolbar dropdowns #51154

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.

Sign up for GitHub

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

Merged
ciampo merged 19 commits into from
Jun 8, 2023
Merged

Use internal context system to apply toolbar variant to toolbar dropdowns #51154

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a11b1ce
DropdownMenu: read variant from context
ciampo May 31, 2023
2e4d820
Toolbar: do not set `variant` prop on popover (rely on context system…
ciampo May 31, 2023
e66a239
Use named export for better storybook doc generation
ciampo May 31, 2023
ea25c42
Provide context value in more toolbar components
ciampo Jun 1, 2023
69576a0
Hook Dropdown into context system, update DropdownMenu and CircularOp…
ciampo Jun 1, 2023
46d70de
Remove toolbar variant popover prop where the context system can be u…
ciampo Jun 1, 2023
fa9892c
Add comment where the context system currently doesn't work
ciampo Jun 1, 2023
1937f31
Use placement prop instead of legacy "position" prop
ciampo Jun 1, 2023
a1b00c7
Add some temporary TODO comments
ciampo Jun 1, 2023
1cecd36
Set toolbar variant for Dropdown in the context system
ciampo Jun 1, 2023
b40411e
Remove explicit variant prop from block alignment matrix control
ciampo Jun 1, 2023
adcd28c
Toolbar: Only set context in the top-level component
ciampo Jun 6, 2023
d3b23ce
Remove `toolbar` variant from instances using `Dropdown`
ciampo Jun 6, 2023
1863132
Simplify Dropdown implementation
ciampo Jun 6, 2023
599f846
Tidy up types
ciampo Jun 6, 2023
40adcd6
Remove toolbar variant storybook example for Popover
ciampo Jun 6, 2023
9ca6589
CHANGELOG
ciampo Jun 6, 2023
59d21e0
Complete renaming internal type
ciampo Jun 6, 2023
af98989
Remove duplicate hardcoded classnames(already added by context connexct)
ciampo Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Remove duplicate hardcoded classnames(already added by context connexct)
  • Loading branch information
@ciampo
ciampo committed Jun 7, 2023
commit af98989a6008f421d6bfb8e46bcae717a42e408a
2 changes: 1 addition & 1 deletion packages/components/src/dropdown-menu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {

return (
<Dropdown
className={ classnames( 'components-dropdown-menu', className ) }
className={ className }
Copy link
Contributor Author

@ciampo ciampo Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The context system automatically generates a components-* classname based on which namespace is passed in the useContextSystem hook — therefore making the components-dropdown-menu hardcoded classname unnecessary, since duplicated.

The same change was applied to the Dropdown component

popoverProps={ mergedPopoverProps }
renderToggle={ ( { isOpen, onToggle } ) => {
const openOnArrowDown = ( event: React.KeyboardEvent ) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/dropdown/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const UnconnectedDropdown = (

return (
<div
className={ classnames( 'components-dropdown', className ) }
className={ className }
ref={ useMergeRefs( [
containerRef,
forwardedRef,
Expand Down