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

[Data Views]: Add drop down with action in headers #55293

Merged
merged 1 commit into from
Oct 12, 2023
Merged

[Data Views]: Add drop down with action in headers #55293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 59 additions & 14 deletions packages/edit-site/src/components/dataviews/list-view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,76 @@ import { flexRender } from '@tanstack/react-table';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { chevronDown, chevronUp } from '@wordpress/icons';
import { Button } from '@wordpress/components';
import { chevronDown, chevronUp, unseen } from '@wordpress/icons';
import {
Button,
Icon,
privateApis as componentsPrivateApis,
} from '@wordpress/components';
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';
import { FieldSortingItems } from './view-actions';

const {
DropdownMenuV2,
DropdownMenuGroupV2,
DropdownMenuItemV2,
DropdownMenuSeparatorV2,
} = unlock( componentsPrivateApis );

const sortIcons = { asc: chevronUp, desc: chevronDown };
function Header( { header } ) {
function HeaderMenu( { dataView, header } ) {
if ( header.isPlaceholder ) {
return null;
}
const text = flexRender(
header.column.columnDef.header,
header.getContext()
);
if ( ! header.column.getCanSort() ) {
const isSortable = !! header.column.getCanSort();
const isHidable = !! header.column.getCanHide();
if ( ! isSortable && ! isHidable ) {
return text;
}
const sortDirection = header.column.getIsSorted();
return (
<Button
onClick={ header.column.getToggleSortingHandler() }
icon={ sortIcons[ sortDirection ] }
iconPosition="right"
text={ text }
style={ { padding: 0 } }
/>
<DropdownMenuV2
align="start"
trigger={
<Button
icon={ sortIcons[ header.column.getIsSorted() ] }
iconPosition="right"
text={ text }
style={ { padding: 0 } }
/>
}
>
{ isSortable && (
<DropdownMenuGroupV2>
<FieldSortingItems
field={ header.column }
dataView={ dataView }
/>
</DropdownMenuGroupV2>
) }
{ isSortable && isHidable && <DropdownMenuSeparatorV2 /> }
{ isHidable && (
<DropdownMenuItemV2
prefix={ <Icon icon={ unseen } /> }
onSelect={ ( event ) => {
event.preventDefault();
header.column.getToggleVisibilityHandler()( event );
} }
>
{ __( 'Hide' ) }
</DropdownMenuItemV2>
) }
</DropdownMenuV2>
);
}

function ListView( { dataView, className, isLoading = false }, ref ) {
const { rows } = dataView.getRowModel();
const hasRows = !! rows?.length;
Expand Down Expand Up @@ -66,7 +108,10 @@ function ListView( { dataView, className, isLoading = false }, ref ) {
.maxWidth || undefined,
} }
>
<Header header={ header } />
<HeaderMenu
dataView={ dataView }
header={ header }
/>
</th>
) ) }
</tr>
Expand Down
64 changes: 29 additions & 35 deletions packages/edit-site/src/components/dataviews/view-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,31 @@ const sortingItemsInfo = {
asc: { icon: arrowUp, label: __( 'Sort ascending' ) },
desc: { icon: arrowDown, label: __( 'Sort descending' ) },
};
export function FieldSortingItems( { field, dataView } ) {
const sortedDirection = field.getIsSorted();
return Object.entries( sortingItemsInfo ).map( ( [ direction, info ] ) => (
<DropdownMenuItemV2
key={ direction }
prefix={ <Icon icon={ info.icon } /> }
suffix={ sortedDirection === direction && <Icon icon={ check } /> }
onSelect={ ( event ) => {
event.preventDefault();
if ( sortedDirection === direction ) {
dataView.resetSorting();
} else {
dataView.setSorting( [
{
id: field.id,
desc: direction === 'desc',
},
] );
}
} }
>
{ info.label }
</DropdownMenuItemV2>
) );
}
function SortMenu( { dataView } ) {
const sortableFields = dataView
.getAllColumns()
Expand All @@ -170,7 +195,6 @@ function SortMenu( { dataView } ) {
}
>
{ sortableFields?.map( ( field ) => {
const sortedDirection = field.getIsSorted();
return (
<DropdownSubMenuV2
key={ field.id }
Expand All @@ -183,40 +207,10 @@ function SortMenu( { dataView } ) {
}
side="left"
>
{ Object.entries( sortingItemsInfo ).map(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the change in this file necessary. Asking because I'm updating this code in #55294
What was changed exactly here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing you just want to reuse the "submenu" for a field in the other PR. I think we need to choose between:

  • Not reusing this component and just duplicating it because one can be independent from tanstack but the other should not.
  • Reusing the one that is not dependent on tanstack.

But since the parent components of both have different requirements, I might think that it's better to not reuse here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just reusing the sort option menu items that are identical in terms of UI. I get the refactoring in your PR, but I don't get how they are different. The both depend on the view config. Maybe the could live in a different file? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

The reusable thing is menu items but the whole menu is different which makes me think it's a bit weird to reuse but it's fine. I can handle that. I changed the props of the component (not use dataView object)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean, it's an easy update to duplicate or reuse and change approach later on. I have no strong opinions. Maybe we should land your PR first and see then. Though I don't think it makes much difference at this point. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'm not too concerned either, whatever lands first is fine :)

( [ direction, info ] ) => {
return (
<DropdownMenuItemV2
key={ direction }
prefix={ <Icon icon={ info.icon } /> }
suffix={
sortedDirection === direction && (
<Icon icon={ check } />
)
}
onSelect={ ( event ) => {
event.preventDefault();
if (
sortedDirection === direction
) {
dataView.resetSorting();
} else {
dataView.setSorting( [
{
id: field.id,
desc:
direction ===
'desc',
},
] );
}
} }
>
{ info.label }
</DropdownMenuItemV2>
);
}
) }
<FieldSortingItems
field={ field }
dataView={ dataView }
/>
</DropdownSubMenuV2>
);
} ) }
Expand Down
Loading