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

[Data views]: Update icons and design tweaks #55391

Merged
merged 7 commits into from
Oct 30, 2023
Merged

[Data views]: Update icons and design tweaks #55391

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d01f011
[Data views]: Update icons and design tweaks
ntsekouras Oct 16, 2023
06da136
Button variants
jameskoster Oct 19, 2023
4b26a38
tooltips
jameskoster Oct 19, 2023
cc3a469
spacing
jameskoster Oct 19, 2023
8d04537
fix pagination from conflicts
ntsekouras Oct 26, 2023
2caf1f9
pagination at the bottom of the frame
ntsekouras Oct 26, 2023
14767da
fix styles
ntsekouras Oct 30, 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: 1 addition & 1 deletion packages/edit-site/src/components/dataviews/dataviews.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default function DataViews( {
}, [ fields ] );
return (
<div className="dataviews-wrapper">
<VStack spacing={ 4 }>
<VStack spacing={ 4 } justify="flex-start">
<HStack>
<HStack justify="start">
<Filters
Expand Down
127 changes: 53 additions & 74 deletions packages/edit-site/src/components/dataviews/pagination.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,11 @@ import {
__experimentalHStack as HStack,
__experimentalText as Text,
__experimentalNumberControl as NumberControl,
__experimentalInputControlPrefixWrapper as InputControlPrefixWrapper,
SelectControl,
} from '@wordpress/components';
import { createInterpolateElement } from '@wordpress/element';
import { sprintf, __, _x, _n } from '@wordpress/i18n';
import { chevronRight, chevronLeft, previous, next } from '@wordpress/icons';

const PAGE_SIZE_VALUES = [ 5, 20, 50 ];
function PageSizeControl( { view, onChangeView } ) {
const label = __( 'Rows per page:' );
return (
<SelectControl
__nextHasNoMarginBottom
label={ label }
hideLabelFromVision
// TODO: This should probably use a label based on the wanted design
// and we could remove InputControlPrefixWrapper usage.
prefix={
<InputControlPrefixWrapper
as="span"
className="dataviews__select-control-prefix"
>
{ label }
</InputControlPrefixWrapper>
}
value={ view.perPage }
options={ PAGE_SIZE_VALUES.map( ( pageSize ) => ( {
value: pageSize,
label: pageSize,
} ) ) }
onChange={ ( value ) =>
onChangeView( { ...view, perPage: value, page: 1 } )
}
/>
);
}

// For now this is copied from the patterns list Pagination component, because
// the datatable pagination starts from index zero(`0`). Eventually all lists will be
// using this one.
function Pagination( {
view,
onChangeView,
Expand All @@ -71,29 +37,37 @@ function Pagination( {
}
</Text>
{ !! totalItems && (
<HStack expanded={ false } spacing={ 1 }>
<Button
variant="tertiary"
onClick={ () => onChangeView( { ...view, page: 1 } ) }
disabled={ view.page === 1 }
aria-label={ __( 'First page' ) }
>
«
</Button>
<Button
variant="tertiary"
onClick={ () =>
onChangeView( { ...view, page: view.page - 1 } )
}
disabled={ view.page === 1 }
aria-label={ __( 'Previous page' ) }
>
‹
</Button>
<HStack expanded={ false } spacing={ 3 }>
<HStack
justify="flex-start"
expanded={ false }
spacing={ 1 }
>
<Button
onClick={ () =>
onChangeView( { ...view, page: 1 } )
}
disabled={ view.page === 1 }
label={ __( 'First page' ) }
icon={ previous }
showTooltip
size="compact"
/>
<Button
onClick={ () =>
onChangeView( { ...view, page: view.page - 1 } )
}
disabled={ view.page === 1 }
label={ __( 'Previous page' ) }
icon={ chevronLeft }
showTooltip
size="compact"
/>
</HStack>
<HStack
justify="flex-start"
expanded={ false }
spacing={ 2 }
>
{ createInterpolateElement(
sprintf(
Expand Down Expand Up @@ -130,29 +104,34 @@ function Pagination( {
}
) }
</HStack>
<Button
variant="tertiary"
onClick={ () =>
onChangeView( { ...view, page: view.page + 1 } )
}
disabled={ view.page >= totalPages }
aria-label={ __( 'Next page' ) }
>
›
</Button>
<Button
variant="tertiary"
onClick={ () =>
onChangeView( { ...view, page: totalPages } )
}
disabled={ view.page >= totalPages }
aria-label={ __( 'Last page' ) }
<HStack
justify="flex-start"
expanded={ false }
spacing={ 1 }
>
»
</Button>
<Button
onClick={ () =>
onChangeView( { ...view, page: view.page + 1 } )
}
disabled={ view.page >= totalPages }
label={ __( 'Next page' ) }
icon={ chevronRight }
showTooltip
size="compact"
/>
<Button
onClick={ () =>
onChangeView( { ...view, page: totalPages } )
}
disabled={ view.page >= totalPages }
label={ __( 'Last page' ) }
icon={ next }
showTooltip
size="compact"
/>
</HStack>
</HStack>
) }
<PageSizeControl view={ view } onChangeView={ onChangeView } />
</HStack>
);
}
Expand Down
15 changes: 10 additions & 5 deletions packages/edit-site/src/components/dataviews/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
.dataviews-wrapper {
width: 100%;
min-height: calc(100% - 60px);
Copy link
Contributor

Choose a reason for hiding this comment

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

The 60px is not part of the data views component, so it seems it's not something we should be applying to .dataviews-wrapper instead maybe it should apply to the current usage of DataViews in the page-pages component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Actually it seem I had this change before the overflow rule. It seems the calc is not needed any more.

overflow: auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this one necessary?

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 think so, yes. Without it, if the list is bigger than the frame's height, it will still scroll but not get the right padding applied to this container and results in having the pagination at the bottom without it. I'm not sure if I could use a better css rule..

padding: $grid-unit-40;

> div {
min-height: 100%;
}
}

.dataviews-pagination {
margin-top: auto;
}

.dataviews-list-view {
Expand Down Expand Up @@ -30,11 +40,6 @@
}
}

.dataviews__select-control-prefix {
color: $gray-700;
text-wrap: nowrap;
}

.dataviews-view-grid__media {
width: 100%;
min-height: 200px;
Expand Down
Loading