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.

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

Jump to bottom

Refactor experimental dropdown menu usages to latest version #55625

Merged
merged 61 commits into from
Dec 21, 2023
Merged

Refactor experimental dropdown menu usages to latest version #55625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
34d2ba1
First attempt at refactoring the view actions dropdown
ciampo Oct 25, 2023
46a5bf5
note
ciampo Oct 25, 2023
be88dba
Force all menus to have "left-start" placement.
ciampo Nov 8, 2023
377b0d8
Port changes missed in rebase due to file renames
oandregal Dec 1, 2023
9432e65
ViewActions: remove unnecessary icon management
oandregal Dec 1, 2023
ee4d695
ViewTable: remove SubMenu, SubMenuTrigger, and use RadioItem
oandregal Dec 1, 2023
b3575f2
Use Ariakit versions in FilterSummary as well
oandregal Dec 1, 2023
b3179a8
AddFilter: migrate to Ariakit
oandregal Dec 1, 2023
a74c886
Update sort behavior to match RadioItem (cannot be unset once set)
oandregal Dec 1, 2023
7530ed4
Remove old add-filter file
oandregal Dec 14, 2023
b8698ab
Use ariakit components
oandregal Dec 14, 2023
f50a1b6
remove submenu trigger
oandregal Dec 14, 2023
6b25c2a
DropdownMenuItem: substitute onSelect by onClick
oandregal Dec 14, 2023
8cedafb
Remove old filter-summary file
oandregal Dec 14, 2023
3b21c2f
FilterSummary: use the ariakit versions
oandregal Dec 14, 2023
e28f4c9
Substitute trigger by menuitem
oandregal Dec 14, 2023
82d89f8
DropdownMenuItem: substitute onSelect by onClick
oandregal Dec 14, 2023
b6ba575
Remove old view-table file
oandregal Dec 14, 2023
ff2e74e
ViewTable: update to ariakit
oandregal Dec 14, 2023
fe549a9
Remove trigger
oandregal Dec 14, 2023
218652c
DropdownMenuItem: substitute onSelect by onClick
oandregal Dec 14, 2023
9e854fe
Remove DropdownSubMenu in favor of DropdownMenu across dataviews package
oandregal Dec 14, 2023
fb23128
Remove suffix chevronrightsmall across dataviews
oandregal Dec 14, 2023
2e7855d
Fix rebase for view-actions
oandregal Dec 14, 2023
5c4e940
Remove unnecessary label
ciampo Dec 14, 2023
2971e40
Use DropdownMenuItemLabel component
ciampo Dec 14, 2023
b5c4599
Hide suffix contents to assistive tech
ciampo Dec 14, 2023
7ac661b
Refactor conditions to use radio menu items
ciampo Dec 14, 2023
087a047
Hide suffix contents to assistive technology
ciampo Dec 15, 2023
a7f3db3
Fix checked check
ciampo Dec 15, 2023
37367ee
Remove unnecessary preventDefault() calls
ciampo Dec 15, 2023
0edbc42
Improve isActive check, remove extra icons, remove extra preventDefau…
ciampo Dec 15, 2023
c7693ab
Fix label not appearing
ciampo Dec 15, 2023
171b13a
Use filter name instead of field, as done for other menus in the file
ciampo Dec 15, 2023
b71ec66
Rename isActive to isChecked
ciampo Dec 15, 2023
a5dd424
Remove unneeded radio semantics from hide button
ciampo Dec 15, 2023
39e26a5
Refactor to DropdownMenuRadioItem
ciampo Dec 15, 2023
f4337f3
Refactor to DrodownMenuRadioItem
ciampo Dec 15, 2023
5a267cf
Refactor OPERATORS
ciampo Dec 15, 2023
e6420b0
Refactor filter summery operators to radio items
ciampo Dec 15, 2023
105063f
Fix OPERATORS object && checks
ciampo Dec 15, 2023
27b69b0
Remove comment
ciampo Dec 15, 2023
2412ab0
Refactor operators code in view table
ciampo Dec 15, 2023
49b8ab8
Add custom spacing for custom menu radio items
ciampo Dec 18, 2023
04adf97
Remove comment
ciampo Dec 19, 2023
d82b7c8
Replace preventDefault with hideOnClick={false}
ciampo Dec 20, 2023
662e7f0
Move OPERATORS object to common constants file
ciampo Dec 20, 2023
ac60750
Use the `onChange` event instead of `onClick` on radio items + the ev…
ciampo Dec 20, 2023
5bd5f4d
Extract custom dropdown radio item implementation
ciampo Dec 20, 2023
eb7abf5
Swap actual menu radio items with custom implementation
ciampo Dec 20, 2023
ae88946
Fix warning when onChange prop is not defined
ciampo Dec 20, 2023
4c7a592
Add a few min widths to dropdowns to avoid jumping and for better whi…
ciampo Dec 20, 2023
6a17c1c
Extract sorting directions to constants
ciampo Dec 20, 2023
17475d9
Refactor more code to use the OPERATORS constant instead of individua…
ciampo Dec 20, 2023
18f3885
Remove un-needed workaround, now that Truncate has been updated
ciampo Dec 21, 2023
09c6a25
Remove hardcoded left placements
ciampo Dec 21, 2023
ffa0e58
Use dot icon instead of check for custom radios
ciampo Dec 21, 2023
36261ed
Use real DropdownMenuRadioItem where possible
ciampo Dec 21, 2023
f7e6b25
Fix e2e test
ciampo Dec 21, 2023
95718a0
Sort dependencies
ciampo Dec 21, 2023
79bff91
Update package.lock
ciampo Dec 21, 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
2 changes: 2 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/dataviews/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@wordpress/i18n": "file:../i18n",
"@wordpress/icons": "file:../icons",
"@wordpress/keycodes": "file:../keycodes",
"@wordpress/primitives": "file:../primitives",
"@wordpress/private-apis": "file:../private-apis",
"classnames": "^2.3.1",
"remove-accents": "^0.5.0"
Expand Down
240 changes: 104 additions & 136 deletions packages/dataviews/src/add-filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,25 @@
import {
privateApis as componentsPrivateApis,
Button,
Icon,
} from '@wordpress/components';
import { chevronRightSmall, funnel, check } from '@wordpress/icons';
import { funnel } from '@wordpress/icons';
import { __, sprintf } from '@wordpress/i18n';
import { Children, Fragment } from '@wordpress/element';

/**
* Internal dependencies
*/
import { unlock } from './lock-unlock';
import { LAYOUT_LIST, OPERATOR_IN, OPERATOR_NOT_IN } from './constants';
import { LAYOUT_LIST, OPERATORS } from './constants';
import { DropdownMenuRadioItemCustom } from './dropdown-menu-helper';

const {
DropdownMenuV2: DropdownMenu,
DropdownMenuGroupV2: DropdownMenuGroup,
DropdownSubMenuV2: DropdownSubMenu,
DropdownSubMenuTriggerV2: DropdownSubMenuTrigger,
DropdownMenuItemV2: DropdownMenuItem,
DropdownMenuSeparatorV2: DropdownMenuSeparator,
DropdownMenuV2Ariakit: DropdownMenu,
DropdownMenuGroupV2Ariakit: DropdownMenuGroup,
DropdownMenuItemV2Ariakit: DropdownMenuItem,
DropdownMenuRadioItemV2Ariakit: DropdownMenuRadioItem,
DropdownMenuSeparatorV2Ariakit: DropdownMenuSeparator,
DropdownMenuItemLabelV2Ariakit: DropdownMenuItemLabel,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird as a component, why not a prop of DropdownMenuItem ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DropdownMenuItemLabel (together witn DropdownMenuItemHelpText) are useful to use the correct typography, and to make sure that the text gets truncated appropriately (label after one line, help text after two lines).

The reason why I exposed them as separate components is because I wanted to give consumers of DropdownMenu the freedom of passing any React component as children. Folks can opt in to the "default" look by using DropdownMenuItemLabel and/or DropdownMenuItemHelpText`, but they can also pass any other content in case they want to go custom.

Using dedicated props instead of children would either limit the flexibility of what we can display in the menu item (ie. what if folks don't want to truncate the label text after 1 line?), or even worse, end up in a similar situation to Button where we currently have a text prop and a children prop.

Having said that, the component is still under active development and only exposed via private APIs, so we can make changes as we see fit.

} = unlock( componentsPrivateApis );

function WithSeparators( { children } ) {
Expand Down Expand Up @@ -50,7 +50,6 @@ export default function AddFilter( { filters, view, onChangeView } ) {

return (
<DropdownMenu
label={ __( 'Filters' ) }
trigger={
<Button
__experimentalIsFocusable
Expand All @@ -66,6 +65,9 @@ export default function AddFilter( { filters, view, onChangeView } ) {
) : null }
</Button>
}
style={ {
minWidth: '230px',
} }
>
<WithSeparators>
<DropdownMenuGroup>
Expand All @@ -82,31 +84,29 @@ export default function AddFilter( { filters, view, onChangeView } ) {
const activeOperator =
filterInView?.operator || filter.operators[ 0 ];
return (
<DropdownSubMenu
<DropdownMenu
key={ filter.field }
trigger={
<DropdownSubMenuTrigger
<DropdownMenuItem
suffix={
<>
{ activeElement &&
activeOperator ===
OPERATOR_IN &&
__( 'Is' ) }
{ activeElement &&
activeOperator ===
OPERATOR_NOT_IN &&
__( 'Is not' ) }
{ activeElement && ' ' }
{ activeElement?.label }
<Icon
icon={ chevronRightSmall }
/>
</>
activeElement && (
<span aria-hidden="true">
{ activeOperator in
OPERATORS &&
`${ OPERATORS[ activeOperator ].label } ` }
{ activeElement.label }
</span>
)
}
>
{ filter.name }
</DropdownSubMenuTrigger>
<DropdownMenuItemLabel>
{ filter.name }
</DropdownMenuItemLabel>
</DropdownMenuItem>
}
style={ {
minWidth: '200px',
} }
>
<WithSeparators>
<DropdownMenuGroup>
Expand All @@ -115,19 +115,12 @@ export default function AddFilter( { filters, view, onChangeView } ) {
activeElement?.value ===
element.value;
return (
<DropdownMenuItem
<DropdownMenuRadioItemCustom
key={ element.value }
role="menuitemradio"
aria-checked={ isActive }
prefix={
isActive && (
<Icon
icon={ check }
/>
)
}
onSelect={ ( event ) => {
event.preventDefault();
name={ `add-filter-${ filter.field.id }` }
oandregal marked this conversation as resolved.
Show resolved Hide resolved
value={ element.value }
checked={ isActive }
onClick={ () => {
onChangeView( {
...view,
page: 1,
Expand All @@ -145,106 +138,77 @@ export default function AddFilter( { filters, view, onChangeView } ) {
} );
} }
>
{ element.label }
</DropdownMenuItem>
<DropdownMenuItemLabel>
{ element.label }
</DropdownMenuItemLabel>
</DropdownMenuRadioItemCustom>
);
} ) }
</DropdownMenuGroup>
{ filter.operators.length > 1 && (
<DropdownSubMenu
<DropdownMenu
trigger={
<DropdownSubMenuTrigger
<DropdownMenuItem
suffix={
<>
{ activeOperator ===
OPERATOR_IN &&
__( 'Is' ) }
{ activeOperator ===
OPERATOR_NOT_IN &&
__( 'Is not' ) }
<Icon
icon={
chevronRightSmall
}
/>{ ' ' }
</>
<span aria-hidden="true">
{
OPERATORS[
activeOperator
]?.label
}
</span>
}
>
{ __( 'Conditions' ) }
</DropdownSubMenuTrigger>
<DropdownMenuItemLabel>
{ __( 'Conditions' ) }
</DropdownMenuItemLabel>
</DropdownMenuItem>
}
>
<DropdownMenuItem
key="in-filter"
role="menuitemradio"
aria-checked={
activeOperator ===
OPERATOR_IN
}
prefix={
activeOperator ===
OPERATOR_IN && (
<Icon icon={ check } />
)
}
onSelect={ ( event ) => {
event.preventDefault();
onChangeView( {
...view,
page: 1,
filters: [
...otherFilters,
{
field: filter.field,
operator:
OPERATOR_IN,
value: filterInView?.value,
},
],
} );
} }
>
{ __( 'Is' ) }
</DropdownMenuItem>
<DropdownMenuItem
key="not-in-filter"
role="menuitemradio"
aria-checked={
activeOperator ===
OPERATOR_NOT_IN
}
prefix={
activeOperator ===
OPERATOR_NOT_IN && (
<Icon icon={ check } />
)
}
onSelect={ ( event ) => {
event.preventDefault();
onChangeView( {
...view,
page: 1,
filters: [
...otherFilters,
{
field: filter.field,
operator:
OPERATOR_NOT_IN,
value: filterInView?.value,
},
],
} );
} }
>
{ __( 'Is not' ) }
</DropdownMenuItem>
</DropdownSubMenu>
{ Object.entries( OPERATORS ).map(
( [
operator,
{ label, key },
] ) => (
<DropdownMenuRadioItem
key={ key }
name={ `add-filter-${ filter.name }-conditions` }
value={ operator }
checked={
activeOperator ===
operator
}
onChange={ ( e ) => {
onChangeView( {
...view,
page: 1,
filters: [
...otherFilters,
{
field: filter.field,
operator:
e
.target
.value,
value: filterInView?.value,
},
],
} );
} }
>
<DropdownMenuItemLabel>
{ label }
</DropdownMenuItemLabel>
</DropdownMenuRadioItem>
)
) }
</DropdownMenu>
) }
<DropdownMenuItem
key={ 'reset-filter-' + filter.name }
disabled={ ! activeElement }
onSelect={ ( event ) => {
event.preventDefault();
hideOnClick={ false }
onClick={ () => {
onChangeView( ( currentView ) => ( {
...currentView,
page: 1,
Expand All @@ -257,31 +221,35 @@ export default function AddFilter( { filters, view, onChangeView } ) {
} ) );
} }
>
{ sprintf(
/* translators: 1: Filter name. e.g.: "Reset Author". */
__( 'Reset %1$s' ),
filter.name.toLowerCase()
) }
<DropdownMenuItemLabel>
{ sprintf(
/* translators: 1: Filter name. e.g.: "Reset Author". */
__( 'Reset %1$s' ),
filter.name.toLowerCase()
) }
</DropdownMenuItemLabel>
</DropdownMenuItem>
</WithSeparators>
</DropdownSubMenu>
</DropdownMenu>
);
} ) }
</DropdownMenuGroup>
<DropdownMenuItem
disabled={
view.search === '' && view.filters?.length === 0
}
onSelect={ ( event ) => {
event.preventDefault();
hideOnClick={ false }
onClick={ () => {
onChangeView( ( currentView ) => ( {
...currentView,
page: 1,
filters: [],
} ) );
} }
>
{ __( 'Reset filters' ) }
<DropdownMenuItemLabel>
{ __( 'Reset filters' ) }
</DropdownMenuItemLabel>
</DropdownMenuItem>
</WithSeparators>
</DropdownMenu>
Expand Down
16 changes: 16 additions & 0 deletions packages/dataviews/src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,22 @@ export const ENUMERATION_TYPE = 'enumeration';
// Filter operators.
export const OPERATOR_IN = 'in';
export const OPERATOR_NOT_IN = 'notIn';
export const OPERATORS = {
[ OPERATOR_IN ]: {
key: 'in-filter',
label: __( 'Is' ),
},
[ OPERATOR_NOT_IN ]: {
key: 'not-in-filter',
label: __( 'Is not' ),
},
};

// Sorting
export const SORTING_DIRECTIONS = {
asc: { label: __( 'Sort ascending' ) },
desc: { label: __( 'Sort descending' ) },
};

// View layouts.
export const LAYOUT_TABLE = 'table';
Expand Down
Loading
Loading