DataViews: Update the data views component to pass an view object #55154
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
youknowriad
added
[Type] Code Quality
youknowriad
commented
Oct 9, 2023
| isLoading, | ||
| paginationInfo, | ||
| options, | ||
| options: { pageCount }, |
There was a problem hiding this comment.
My goal is to actually eliminate this prop and the paginationInfo one. The remaining information should probably be part of "data" but I'll think about these separately, the current PR just focuses on the "view config".
youknowriad
commented
Oct 9, 2023
youknowriad
commented
Oct 9, 2023
youknowriad
commented
Oct 9, 2023
| search: '', | ||
| page: 0, | ||
| perPage: 5, | ||
| sort: { |
There was a problem hiding this comment.
Ultimately, we may want to support an array here but let's keep the existing behavior for now.
There was a problem hiding this comment.
Probably better to rename column->field
There was a problem hiding this comment.
What do you think about #55205 ?
|
Size Change: +1.21 kB (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
youknowriad
commented
Oct 9, 2023
| sort: { | ||
| column: 'date', | ||
| direction: 'desc', | ||
| }, |
There was a problem hiding this comment.
Right now, the visible columns is an internal property, I think we may want to add this to this object later.
There was a problem hiding this comment.
Yes. Fields visibility should be tied to a view and in the future also probably handle some more meta like visibility in mobile.
There was a problem hiding this comment.
There are also some field metadata related to UI that could also be part of the view. For example, we currently set maxWidth via the fields (and there could be other widths according to the only docs I could find).
I'm thinking of a situation where the views are customized per user/role/etc. or even updated real-time in collaborative editing. In that scenario, the width of the columns seems like an important UI detail to maintain in the view entity – so it can be updated/synchronized/etc.
youknowriad
commented
Oct 9, 2023
|
Flaky tests detected in b582669. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6455335861
|
ntsekouras
reviewed
Oct 9, 2023
ntsekouras
reviewed
Oct 9, 2023
| } ) { | ||
| const dataView = useReactTable( { | ||
| data, | ||
| columns: fields, | ||
| ...options, | ||
| manualSorting: true, |
There was a problem hiding this comment.
For now we can add these internally(manualSorting and the rest manual props), but if we feed the component with static data, these should not be set to true values.
There was a problem hiding this comment.
Yeah, I'd like for these to remain internal implementation detail, we may have a way to check whether the data is static or not depending on the "data" props we provide later.
| @@ -84,6 +73,9 @@ export default function PagePages() { | |||
| method: 'HEAD', | |||
| parse: false, | |||
| } ).then( ( res ) => { | |||
| // TODO: store this in core-data reducer and | |||
There was a problem hiding this comment.
@youknowriad is there an open issue for this to follow?
There was a problem hiding this comment.
No, no issue at the moment, I was thinking about adding a todo list item to the data views overview issue
There was a problem hiding this comment.
This should be addressed in #55164
ntsekouras
reviewed
Oct 9, 2023
ntsekouras
reviewed
Oct 9, 2023
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.
Related to #55083
What?
This is a small refactoring PR that updates the new Pages list in the site editor extracts all the config of the view into a single object. This will allow us down the road to persist this object in the server if needed.
Testing Instructions
1- Open Site Editor > Pages > Manage all pages
2- Play with the UI: filtering, sorting, pagination...