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: Add ability to display fields as a badge in grid layout #60284

Merged
merged 14 commits into from
Apr 5, 2024
Merged

Data views: Add ability to display fields as a badge in grid layout #60284

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f2a20f5
first draft
jameskoster Mar 27, 2024
6415a4b
separate badge fields from others
jameskoster Mar 27, 2024
50a3721
Additional logic and styling
jameskoster Mar 28, 2024
1a4c96a
Add field id class to badge, and style sync status
jameskoster Mar 28, 2024
9d14dd5
small refactoring
ntsekouras Apr 1, 2024
0e2fc17
feedback
ntsekouras Apr 2, 2024
89f8a80
update css classnames
ntsekouras Apr 2, 2024
4998c22
Remove separator
jameskoster Apr 2, 2024
8097196
Page field display
jameskoster Apr 2, 2024
8b79460
Merge branch 'trunk' into add/dataviews-grid-display-field-as-badge
jameskoster Apr 4, 2024
4926501
Remove 'is-badge' class
jameskoster Apr 4, 2024
1c038c5
lint
jameskoster Apr 4, 2024
4fc6e50
Revert date field label change
jameskoster Apr 4, 2024
ef9a806
Merge branch 'trunk' into add/dataviews-grid-display-field-as-badge
jameskoster Apr 5, 2024
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
28 changes: 27 additions & 1 deletion packages/dataviews/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,9 @@
line-height: 16px;

&:not(:empty) {
padding: $grid-unit-15;
padding: $grid-unit-15 0;
padding-top: 0;
margin: 0 $grid-unit-15;
}

.dataviews-view-grid__field {
Expand All @@ -355,6 +356,31 @@
}
}
}

.dataviews-view-grid__badge-fields {
&:not(:empty) {
padding: $grid-unit-15;
padding-top: 0;
}

.dataviews-view-grid__field-value {
&.is-badge {
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
width: fit-content;
background: $gray-100;
padding: 0 $grid-unit-10;
min-height: $grid-unit-30;
border-radius: $radius-block-ui;
display: flex;
align-items: center;
font-size: 12px;
}
}

&:not(:empty) + .dataviews-view-grid__fields:not(:empty) {
padding-top: $grid-unit-15;
border-top: 1px solid $gray-100;
}
}
}

.dataviews-view-list {
Expand Down
133 changes: 91 additions & 42 deletions packages/dataviews/src/view-grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ function GridItem( {
mediaField,
primaryField,
visibleFields,
badgeFields,
displayAsColumnFields,
} ) {
const hasBulkAction = useHasAPossibleBulkAction( actions, item );
Expand Down Expand Up @@ -100,46 +101,80 @@ function GridItem( {
</HStack>
<ItemActions item={ item } actions={ actions } isCompact />
</HStack>
<VStack className="dataviews-view-grid__fields" spacing={ 3 }>
{ visibleFields.map( ( field ) => {
const renderedValue = field.render( {
item,
} );
if ( ! renderedValue ) {
return null;
}
return (
<Flex
className={ classnames(
'dataviews-view-grid__field',
displayAsColumnFields?.includes( field.id )
? 'is-column'
: 'is-row'
) }
key={ field.id }
gap={ 1 }
justify="flex-start"
expanded
style={ { height: 'auto' } }
direction={
displayAsColumnFields?.includes( field.id )
? 'column'
: 'row'
}
>
<FlexItem className="dataviews-view-grid__field-name">
{ field.header }
</FlexItem>
{ !! badgeFields?.length && (
<HStack
className="dataviews-view-grid__badge-fields"
spacing={ 2 }
wrap
align="top"
justify="flex-start"
>
{ badgeFields.map( ( field ) => {
const renderedValue = field.render( {
item,
} );
if ( ! renderedValue ) {
return null;
}
return (
<FlexItem
className="dataviews-view-grid__field-value"
style={ { maxHeight: 'none' } }
key={ field.id }
className={ classnames(
'dataviews-view-grid__field-value',
`dataviews-view-grid__field-${ field.id }`,
'is-badge'
Copy link
Member

Choose a reason for hiding this comment

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

It sounds like we may remove the is-badge class? It's only used in packages/edit-site/src/components/page-patterns/style.scss but it seems to be unnecessary?

) }
>
{ renderedValue }
</FlexItem>
</Flex>
);
} ) }
</VStack>
);
} ) }
</HStack>
) }
{ !! visibleFields?.length && (
<VStack className="dataviews-view-grid__fields" spacing={ 3 }>
{ visibleFields.map( ( field ) => {
const renderedValue = field.render( {
item,
} );
if ( ! renderedValue ) {
return null;
}
return (
<Flex
className={ classnames(
'dataviews-view-grid__field',
displayAsColumnFields?.includes( field.id )
? 'is-column'
: 'is-row'
) }
key={ field.id }
gap={ 1 }
justify="flex-start"
expanded
style={ { height: 'auto' } }
direction={
displayAsColumnFields?.includes( field.id )
? 'column'
: 'row'
}
>
<>
<FlexItem className="dataviews-view-grid__field-name">
{ field.header }
</FlexItem>
<FlexItem
className="dataviews-view-grid__field-value"
style={ { maxHeight: 'none' } }
>
{ renderedValue }
</FlexItem>
</>
</Flex>
);
} ) }
</VStack>
) }
</VStack>
);
}
Expand All @@ -161,12 +196,25 @@ export default function ViewGrid( {
const primaryField = fields.find(
( field ) => field.id === view.layout.primaryField
);
const visibleFields = fields.filter(
( field ) =>
! view.hiddenFields.includes( field.id ) &&
! [ view.layout.mediaField, view.layout.primaryField ].includes(
field.id
)
const { visibleFields, badgeFields } = fields.reduce(
( accumulator, field ) => {
if (
view.hiddenFields.includes( field.id ) ||
[ view.layout.mediaField, view.layout.primaryField ].includes(
field.id
)
) {
return accumulator;
}
// If the field is a badge field, add it to the badgeFields array
// otherwise add it to the rest visibleFields array.
const key = view.layout.displayAsBadgeFields?.includes( field.id )
? 'badgeFields'
: 'visibleFields';
accumulator[ key ].push( field );
return accumulator;
},
{ visibleFields: [], badgeFields: [] }
);
const shownData = useAsyncList( data, { step: 3 } );
const usedData = deferredRendering ? shownData : data;
Expand Down Expand Up @@ -194,6 +242,7 @@ export default function ViewGrid( {
mediaField={ mediaField }
primaryField={ primaryField }
visibleFields={ visibleFields }
badgeFields={ badgeFields }
displayAsColumnFields={
view.layout.displayAsColumnFields
}
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/page-pages/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ export default function PagePages() {
},
},
{
header: __( 'Date' ),
header: __( 'Publish date' ),
id: 'date',
render: ( { item } ) => {
const formattedDate = dateI18n(
Expand Down
21 changes: 13 additions & 8 deletions packages/edit-site/src/components/page-patterns/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const defaultConfigPerViewType = {
[ LAYOUT_GRID ]: {
mediaField: 'preview',
primaryField: 'title',
displayAsBadgeFields: [ 'sync-status' ],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should declare a badge fields once and has nothing to do with asColumnFields. Let me refactor the code a bit..

Copy link
Member

Choose a reason for hiding this comment

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

In terms of naming, I think we should remove the displayAs prefix everywhere. We already use badgeFields in the UI component below, and there's no difference from the existing primaryField and mediaField (all of this is about "marking" a set of fields and display them differently).

The same applies to columnFields, that can be changed in a different PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds fine to me.

},
};
const DEFAULT_VIEW = {
Expand Down Expand Up @@ -300,19 +301,23 @@ export default function DataviewsPatterns() {
];
if ( type === PATTERN_TYPES.theme ) {
_fields.push( {
header: __( 'Sync Status' ),
header: __( 'Sync status' ),
id: 'sync-status',
render: ( { item } ) => {
// User patterns can have their sync statuses checked directly.
// Non-user patterns are all unsynced for the time being.
return (
SYNC_FILTERS.find(
( { value } ) => value === item.syncStatus
)?.label ||
SYNC_FILTERS.find(
( { value } ) =>
value === PATTERN_SYNC_TYPES.unsynced
).label
<span
className={ `edit-site-patterns__field-sync-status-${ item.syncStatus }` }
>
{ SYNC_FILTERS.find(
( { value } ) => value === item.syncStatus
)?.label ||
SYNC_FILTERS.find(
( { value } ) =>
value === PATTERN_SYNC_TYPES.unsynced
).label }
</span>
);
},
type: ENUMERATION_TYPE,
Expand Down
5 changes: 5 additions & 0 deletions packages/edit-site/src/components/page-patterns/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@
.dataviews-pagination {
z-index: z-index(".edit-site-patterns__dataviews-list-pagination");
}

.dataviews-view-grid__field-sync-status.is-badge:has(.edit-site-patterns__field-sync-status-fully) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer if we don't target dataviews classes in consumer code, as it implies it's a public API. Can we instead add a specific class in the sync-status field? We have access to the view type there.

I've noticed that we do this for pagination, pattern actions, and templates/parts actions. cc @ntsekouras

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I don't think it's bad to use a specific class with a field's id suffix and I wouldn't worry right now about indicating public API, since we can have breaking changes in the DataViews package.

Having said that, I agree with you that we don't need this right now. I updated the code.

background: rgba(var(--wp-block-synced-color--rgb), 0.04);
color: var(--wp-block-synced-color);
}
}

.dataviews-action-modal__duplicate-pattern {
Expand Down
Loading