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 list layout: revise for improved text truncation #65376

Merged
merged 10 commits into from
Oct 14, 2024
Merged

Data Views list layout: revise for improved text truncation #65376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
389718c
Remove background-color and box-shadow from actions
colorful-tones Sep 10, 2024
d3a5105
Shorten primary field’s text on hover or selected state
stokesman Sep 11, 2024
779ae85
Data Views list layout: revise for improved text truncation
stokesman Sep 12, 2024
54645c8
Let primary field be full with in resting state
stokesman Sep 14, 2024
c933960
Remove seemingly errant border radius on item wrapper
stokesman Sep 14, 2024
dbb1958
Unnest actions gridcells
stokesman Sep 14, 2024
9601c78
Fix cursor style of list items
stokesman Oct 8, 2024
59e8b7d
Update test selector
stokesman Oct 8, 2024
a60c591
Revert "Update test selector"
stokesman Oct 9, 2024
db85c39
Put primary field in front if it has interactive elements
stokesman Oct 9, 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
181 changes: 81 additions & 100 deletions packages/dataviews/src/dataviews-layouts/list/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,9 @@ function ListItem< Item >( {
const descriptionId = `${ idPrefix }-description`;

const [ isHovered, setIsHovered ] = useState( false );
const handleMouseEnter = () => {
setIsHovered( true );
};
const handleMouseLeave = () => {
setIsHovered( false );
const handleHover: React.MouseEventHandler = ( { type } ) => {
const isHover = type === 'mouseenter';
setIsHovered( isHover );
};

useEffect( () => {
Expand Down Expand Up @@ -187,6 +185,45 @@ function ListItem< Item >( {
<primaryField.render item={ item } />
) : null;

const usedActions = eligibleActions?.length > 0 && (
<HStack spacing={ 3 } className="dataviews-view-list__item-actions">
{ primaryAction && (
<PrimaryActionGridCell
idPrefix={ idPrefix }
primaryAction={ primaryAction }
item={ item }
/>
) }
<div role="gridcell">
<DropdownMenu
trigger={
<Composite.Item
id={ generateDropdownTriggerCompositeId(
idPrefix
) }
render={
<Button
size="small"
icon={ moreVertical }
label={ __( 'Actions' ) }
accessibleWhenDisabled
disabled={ ! actions.length }
onKeyDown={ onDropdownTriggerKeyDown }
/>
}
/>
}
placement="bottom-end"
>
<ActionsDropdownMenuGroup
actions={ eligibleActions }
item={ item }
/>
</DropdownMenu>
</div>
</HStack>
);

return (
<Composite.Row
ref={ itemRef }
Expand All @@ -196,116 +233,60 @@ function ListItem< Item >( {
'is-selected': isSelected,
'is-hovered': isHovered,
} ) }
onMouseEnter={ handleMouseEnter }
onMouseLeave={ handleMouseLeave }
onMouseEnter={ handleHover }
onMouseLeave={ handleHover }
>
<HStack
className="dataviews-view-list__item-wrapper"
alignment="center"
spacing={ 0 }
>
<HStack className="dataviews-view-list__item-wrapper" spacing={ 0 }>
<div role="gridcell">
<Composite.Item
render={ <div /> }
role="button"
ciampo marked this conversation as resolved.
Show resolved Hide resolved
id={ generateItemWrapperCompositeId( idPrefix ) }
aria-pressed={ isSelected }
aria-labelledby={ labelId }
aria-describedby={ descriptionId }
className="dataviews-view-list__item"
onClick={ () => onSelect( item ) }
/>
Copy link
Contributor

@ciampo ciampo Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm a bit worried about having a button with no contents, is there any way we could move all cell content (except for the actions buttons) inside it?

Copy link
Contributor Author

@stokesman stokesman Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, this is the bit I wondered most about but hoped the existing aria-labeledby and aria-describedby makes it okay.

move all cell content (except for the actions buttons) inside it

The actions have to be a sibling of the title for the truncation to "just work". So both would have to stay outside the main button, but then they’d not be able to impact the layout of other content. Not to say that can’t be made to work, but then the title is outside the button anyway.

Copy link
Contributor Author

@stokesman stokesman Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did some testing with VoiceOver and it announces the same content as on trunk when navigating the rows and gridcells here. I suppose to be totally sure testing with other screen readers is needed but I expect they all do well as support for aria-labelledby ought to be good.

Copy link
Contributor

Choose a reason for hiding this comment

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

@afercia maybe you could advise if this change is OK in terms of accessibility, or if it has unwanted consequences?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ciampo Technically, it's not that different from an icon-button where the SVG icon is aria-hidden so that the button is 'empty'. However, it's always preferable to not use ARIA when possible. We shouldn't assume all assistive technologies understand ARIA, for example with some combination of browsers and speech recognition software. Is it possible to use visually hidden content instead of aria-label?
Anyways, I'm not sure this is the only problem with the list view implementation and the dataviews in general as an in-depth accessibility audit has still to happen and that's something I can't do alone.

</div>
<HStack spacing={ 3 } justify="start" alignment="flex-start">
<div className="dataviews-view-list__media-wrapper">
{ renderedMediaField }
</div>
<VStack
spacing={ 1 }
className="dataviews-view-list__field-wrapper"
>
<HStack
spacing={ 3 }
justify="start"
alignment="flex-start"
>
<div className="dataviews-view-list__media-wrapper">
{ renderedMediaField }
</div>
<VStack
spacing={ 1 }
className="dataviews-view-list__field-wrapper"
<HStack spacing={ 0 }>
<div
className="dataviews-view-list__primary-field"
id={ labelId }
>
<span
className="dataviews-view-list__primary-field"
id={ labelId }
>
{ renderedPrimaryField }
</span>
{ renderedPrimaryField }
</div>
{ usedActions }
</HStack>
<div
className="dataviews-view-list__fields"
id={ descriptionId }
>
{ visibleFields.map( ( field ) => (
<div
className="dataviews-view-list__fields"
id={ descriptionId }
key={ field.id }
className="dataviews-view-list__field"
>
{ visibleFields.map( ( field ) => (
<div
key={ field.id }
className="dataviews-view-list__field"
>
<VisuallyHidden
as="span"
className="dataviews-view-list__field-label"
>
{ field.label }
</VisuallyHidden>
<span className="dataviews-view-list__field-value">
<field.render item={ item } />
</span>
</div>
) ) }
<VisuallyHidden
as="span"
className="dataviews-view-list__field-label"
>
{ field.label }
</VisuallyHidden>
<span className="dataviews-view-list__field-value">
<field.render item={ item } />
</span>
</div>
</VStack>
</HStack>
</Composite.Item>
</div>
{ eligibleActions?.length > 0 && (
<HStack
spacing={ 3 }
justify="flex-end"
className="dataviews-view-list__item-actions"
style={ {
flexShrink: '0',
width: 'auto',
} }
>
{ primaryAction && (
<PrimaryActionGridCell
idPrefix={ idPrefix }
primaryAction={ primaryAction }
item={ item }
/>
) }
<div role="gridcell">
<DropdownMenu
trigger={
<Composite.Item
id={ generateDropdownTriggerCompositeId(
idPrefix
) }
render={
<Button
size="small"
icon={ moreVertical }
label={ __( 'Actions' ) }
accessibleWhenDisabled
disabled={ ! actions.length }
onKeyDown={
onDropdownTriggerKeyDown
}
/>
}
/>
}
placement="bottom-end"
>
<ActionsDropdownMenuGroup
actions={ eligibleActions }
item={ item }
/>
</DropdownMenu>
) ) }
</div>
</HStack>
) }
</VStack>
</HStack>
</HStack>
</Composite.Row>
);
Expand Down
75 changes: 32 additions & 43 deletions packages/dataviews/src/dataviews-layouts/list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,63 +7,40 @@ ul.dataviews-view-list {

li {
margin: 0;
cursor: pointer;
border-top: 1px solid $gray-100;

.dataviews-view-list__item-wrapper {
position: relative;
border-radius: $grid-unit-05;

> * {
width: 100%;
}
padding: $grid-unit-20 $grid-unit-30;
}

.dataviews-view-list__item-actions {
position: absolute;
top: $grid-unit-20;
right: 0;

flex: 0;
overflow: hidden;

> div {
height: $button-size-small;
}

.components-button {
position: relative;
z-index: 1;
opacity: 0;
}
}

&:has(.dataviews-view-list__fields:empty) {
.dataviews-view-list__item-actions {
top: 50%;
transform: translateY(-50%);
}
}

&.is-selected,
&.is-hovered,
&:focus-within {
&:where(.is-selected, .is-hovered, :focus-within) {
.dataviews-view-list__item-actions {
background: #f8f8f8;
padding-left: $grid-unit-10;
margin-right: $grid-unit-30;
box-shadow: -12px 0 8px 0 #f8f8f8;
flex-basis: min-content;
overflow: unset;
margin-inline: $grid-unit-10 0;

.components-button {
opacity: 1;
position: static;
}
}
}

&.is-selected {
.dataviews-view-list__item-actions {
background-color: rgb(247 248 255);
box-shadow: -12px 0 8px 0 rgb(247 248 255);
}
}

&.is-selected.is-selected {
border-top: 1px solid rgba(var(--wp-admin-theme-color--rgb), 0.12);

Expand Down Expand Up @@ -105,27 +82,38 @@ ul.dataviews-view-list {
}

.dataviews-view-list__item {
box-sizing: border-box;
padding: $grid-unit-20 $grid-unit-30;
width: 100%;
position: absolute;
z-index: 1;
inset: 0;
scroll-margin: $grid-unit-10 0;
appearance: none;
border: none;
background: none;
padding: 0;
cursor: pointer;

&:focus-visible {
outline: none;

&::before {
position: absolute;
content: "";
top: var(--wp-admin-border-width-focus);
right: var(--wp-admin-border-width-focus);
bottom: var(--wp-admin-border-width-focus);
left: var(--wp-admin-border-width-focus);
inset: var(--wp-admin-border-width-focus);
box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color);
border-radius: $radius-small;
// Windows High Contrast mode will show this outline, but not the box-shadow.
outline: 2px solid transparent;
}
}
.dataviews-view-list__primary-field {
min-height: $grid-unit-30;
line-height: $grid-unit-30;
overflow: hidden;
}
.dataviews-view-list__primary-field {
flex: 1;
min-height: $grid-unit-30;
line-height: $grid-unit-30;
overflow: hidden;
// The field should be in front of the main button in case it has a link/button.
&:has(a, button) {
z-index: 1;
}
}

Expand Down Expand Up @@ -164,6 +152,7 @@ ul.dataviews-view-list {

.dataviews-view-list__field-wrapper {
min-height: $grid-unit-05 * 13; // Ensures title is centrally aligned when all fields are hidden
flex-grow: 1;
}

.dataviews-view-list__fields {
Expand Down
Loading