Data views list layout: Re-organise actions display #63043
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jameskoster
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +73 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
This seems overall a great step forward. The only one that gives me pause is this one:
In this one, the actions on the right don't feel well-placed. Is there a way we can keep those right-most actions vertically centered according to the thumbnail? The description would need to not go to the right-most edge, but that seems an improvement regardless. |
|
I couldn't think of a way that doesn't result in other issues. Leaving the actions in their own column will result in the 'jump' issue described above. I tried absolutely positioning the actions, but that causes them to overlap long titles and fields which seems undesirable to me. We also need to consider that in the future there might be data view consumers that do not employ a thumbnail in List layout. That's why aligning to the title makes a bit more sense to me. A different avenue to explore might be tweaking the metrics a bit, to make this placement feel a bit more intentional. Perhaps increasing the spacing: 
Figma here if you'd like to try some options. |
One argument for this approach is that it enables us to include icons in field values, e.g. 
|
|
Thanks for considering the keyboard navigation in your test instructions :) There're some e2e tests for that but it's still nice to do a manual one in case there's something not covered. I've also done it myself and it's working properly. The changes improve the experience, and it's working as expected. I can take a deeper look from a code point of view when design direction is confirmed. I'll be AFK soon, so don't wait for my approval if I'm not around — my major concern here is that keyboard navigation still works fine. |
45 tasks
|
The author field will also soon display the users avatar (#63142), which further indicates we should tweak the layout slightly. I'll work on an update soon. |
|
Connecting another dot; the status field may also be updated to include the icon: #63199. |
|
Updated the metrics a bit: 
Here's how things look when support for icons in field values is added: 
This one is ready for a fresh review. |
|
Keyboard navigation still works correctly as before, and I don't think it would be difficult to resolve the e2e test failure. I think the accessibility issue that needs to be resolved here is applying element roles properly. trunkElements in a row are divided into grid cells according to their role. This PRThere is only one grid cell in a row, and grid cells are nested. Is there any way to improve the layout while still maintaining the original roles and hierarchy? |
|
Thank you for pointing that out. It demonstrates another issue on trunk that this PR fixes; clicking around the action buttons will not select the row, despite the hover style suggesting it would: click.mp4That said, I'm not sure it's possible to retain the layout and avoid incorrect nesting of roles. This could suggest that absolutely positioning the actions is the way to go after all. I'll probably spin up a different PR to try that. |
|
Closing due to #63299 being merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR attempts to address two issues in list layout.
The first is the awkward alignment of the title when all fields are toggled off:
The second is the reflow that occurs when hovering/selecting a row that contains a field with a very long value, or just many fields:
jump.mp4
How?
Currently the title + fields are in one column, and the actions are in another. When the actions column appears, the title + fields column gets squished to accommodate it causing wrapping.
In this PR I've re-organised the markup so that the title and actions are in a row, with fields beneath. When the actions are revealed the title is squished, but truncated to avoid reflow.
To accomplish this I had to increase the height of the title slightly to align with the action icons. This also increased the height of the title + fields container in a two-row configuration by 4px, so the thumbnail was increased to match. This structure is better described in an image:
The result is that regardless of how many fields are visible (or how long the field values are), alignment is maintained:
Testing Instructions