Proof of concept: visualize hierarchical data #66479
Merged
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
oandregal
added
[Status] In Progress
oandregal
requested review from
youknowriad,
mtias,
jameskoster and
jasmussen
|
Size Change: +140 B (+0.01%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
|
Nice. We probably want a nicer indentation line, potentially an SVG that stacks for indentation. For the little chips that indicate parent/children, they should probably use the same design as other similar chips (did we get a component for this yet?), i.e. the proper radius, and horizontal padding. |
|
What's the status here :) Do you think we should pursue this further? |
|
I thought I'd be able to get back to this one yesterday, but catching up with my ping queue after all the trips, etc. has been more demanding than expected. I can probably resume this experiment tomorrow. |
|
Just catching up with this one too. Some points from a separate discussion to consider:
|
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
from
c21c98e to
6228887
Compare
|
Looking into this again. I've reviewed the states we need to consider:
And noticed there're a few aspects to this, according to wp-admin implementation:
This is how wp-admin looks for every one of them: Initial load
PaginatedThe parent ("level 1") of the item ("level 2C") is included in the list, even though it was already displayed in the previous page:
With search
Note how the items aren't grouped under the parent. "About the tests" is the parent to many pages, but not all of them come after it. The pages are sorted in whatever order is using (not sure how: despite what the control says, it's not the title): 
SortedNo indication of hierarchy. 
Filtered
|
|
I think the wp-admin examples are mostly okay, though showing the indentation in search can be a bit misleading. In that case we might consider omitting the hierarchy. Instead of showing the parent as a label, how about showing the 'parent' field instead? For the initial load we still need to work out:
For the second point mimicking wp-admin seems okay. |
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
from
6228887 to
32013a6
Compare
|
I found a direction that can work:
The current PoC only works for a single page, but you can test it by searching, filtering, reordering. The code needs to change a lot, so don't focus too much on specifics but comment on direction. Screen.Recording.2024-11-29.at.12.09.02.movNext steps:
Note that, in this PR, the levels are calculated in the client. We cannot do this because they'd be relative to the dataset that's being displayed. Observe this bug: Screen.Recording.2024-11-29.at.11.25.11.mov |
oandregal
changed the title
|
Flaky tests detected in 28e8449. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12390109624
|
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
from
32013a6 to
bc9f76b
Compare
|
A tiny design detail, but I think we can remove the spaces between each emdash so they appear as single lines. IE:
|
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
2 times, most recently
from
56ae1e4 to
292f8d9
Compare
|
Looked at this today and addressed all of this: pagination now works, there's a view config control to enable/disable hierarchical sort, and levels are calculated in the server. I've also tweaked the level indicator, as per Jay's comment. The dataset is sorted by ascending title: Screen.Recording.2024-12-03.at.17.25.21.movWhat do you all feel about the overall interaction @jasmussen @jameskoster @youknowriad @mtias ? The major thing I see is fixing the implementation of levels in the server, there's a subtle bug when the parent is not part of the dataset:
There's also some thinking to do around dataviews behavior: the view config control, the naming, if the "parent" field behaves like any other field (can be hidden) or is special. If there's no major interactions that need fixing, I'll start looking into those details. |
|
Thanks for the continued effort! Perhaps this is related to the bug you mentioned; if you search for children (so the indentation disappears), then clear the search, the indentation of those items remains hidden in the main list too: indent.mp4Not showing indentation in search results isn't a deal breaker given the 'Parent' field is visible, but I think it needs to be visible in the main list. One other detail to decide on is the options/behavior of the 'Sort by' option. Currently the PR behaves differently to wp-admin as it respects hierarchy when sorting by title. You could argue this means you're not truly sorting by title :) If we wanted to match wp-admin we could try adding a new 'Default' option to the 'Sort by' control. 'Default' would mean sorting by title with hierarchy. If you switch to sort by title then hierarchy disappears and you see pages ordered alphanumerically. What do you think?
I lean towards hidden initially; the indentation already communicates the relationships. It should probably appear conditionally, IE when searching. Random observation while testing, I wonder if the 'create new page' dialog should allow you to select a parent... |
ntsekouras
reviewed
Dec 4, 2024
| @@ -58,10 +64,10 @@ const DEFAULT_POST_BASE = { | |||
| page: 1, | |||
| perPage: 20, | |||
| sort: { | |||
| field: 'date', | |||
| direction: 'desc', | |||
| field: 'title', | |||
There was a problem hiding this comment.
Why change the default sort here?
ntsekouras
reviewed
Dec 4, 2024
ntsekouras
reviewed
Dec 4, 2024
…zation" This reverts commit 2b06468f3f171cdf8a08b10bde7c7e431d230800.
This reverts commit c053e1e.
Feel free to revert if you prefer the former structure, of course.
- Rename $ancestors to $level - Remove $root_not_found, preferring a better `while` condition - Fix linting issues (spacing, and `++$a` instead of `$a++`)
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
from
6ed0eed to
da863ad
Compare
|
@mcsf thanks for the updates! @mcsf @ntsekouras @youknowriad I've pushed changes to address the feedback in the client code. After lunch, I'll review the server and will prepare the corresponding backport PR as well. |
|
Created the backport PR at WordPress/wordpress-develop#8014 and the corresponding trac ticket as well https://core.trac.wordpress.org/ticket/62701 |
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
from
3060bea to
e5dc6f9
Compare
oandregal
force-pushed
the
try/dataviews-hierarchical-data
branch
from
e5dc6f9 to
9cd80ee
Compare
oandregal
commented
Dec 18, 2024
| @@ -395,7 +395,7 @@ test.describe( 'Site Editor Performance', () => { | |||
| await requestUtils.activateTheme( 'twentytwentyfour' ); | |||
| } ); | |||
|
|
|||
| const perPage = 20; | |||
| const perPage = 9; | |||
There was a problem hiding this comment.
The test was failing because it didn't find the Page 8 that it created.
This PR switches the page order from date to title, and the Page 8 was buried in the second page of results. The reason for that is that the REST endpoint returns the titles Page 1x (ten-something) in the first page, moving Page 8 to the second page. Limiting results to 9 fixes this issue.
See how the REST endpoint sorts Page 1, Page 2, Page 11:
mcsf
approved these changes
Dec 18, 2024
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.
Closes #55957
Related #66570
What
Proof of concept to work with hierarchical data in DataViews:
Screen.Recording.2024-12-03.at.17.25.21.mov
How
How to test
Have pages with hierarchy (many levels down) & open with site editor's page (in only works in table layout for now).