DataViews: make dataviews powered page patterns stable #58139
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
[Type] Experimental
youknowriad
approved these changes
Jan 23, 2024
youknowriad
added
[Type] Enhancement
|
Size Change: -1.87 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
oandregal
commented
Jan 23, 2024
| @@ -1,42 +1,421 @@ | |||
| /** | |||
| * WordPress dependencies | |||
| */ | |||
| import { __ } from '@wordpress/i18n'; | |||
There was a problem hiding this comment.
The changes in this file are just copy/pasting dataviews-patterns.js. These can be skipped from the review.
There was a problem hiding this comment.
I was surprised that GitHub didn't say the file moved.
There was a problem hiding this comment.
There are changes in other files (imports), not sure if that's the reason.
oandregal
commented
Jan 23, 2024
| @@ -9,9 +9,8 @@ import { privateApis as routerPrivateApis } from '@wordpress/router'; | |||
| import { unlock } from '../../lock-unlock'; | |||
| import { useIsSiteEditorLoading } from './hooks'; | |||
| import Editor from '../editor'; | |||
| import DataviewsPatterns from '../page-patterns/dataviews-patterns'; | |||
There was a problem hiding this comment.
This file is the only that really changes.
oandregal
requested review from
mcsf,
ellatrix,
jorgefilipecosta and
ntsekouras
oandregal
force-pushed
the
update/grid-for-patterns-stable
branch
from
849a323 to
e339b81
Compare
youknowriad
reviewed
Jan 23, 2024
| } ); | ||
| this.list = this.content.getByRole( 'list' ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Feels like there's an opportunity to test our new screens better. Maybe we should adapt the tests instead?
There was a problem hiding this comment.
Yeah, we need to add tests for every new page. I am unsure if I'll have time for that before RC, but can commit to do it in a follow-up.
There was a problem hiding this comment.
I agree we should adapt the tests. If the plan is to do it a follow up, maybe just skip them for now?
There was a problem hiding this comment.
Sure, I can skip them 4506147 Though I'm skeptical we can reuse any of that code (that's why I removed in the first place).
There was a problem hiding this comment.
I added tasks for adding e2e test for every page in the tracking issue.
oandregal
force-pushed
the
update/grid-for-patterns-stable
branch
from
e339b81 to
a8ef160
Compare
|
There are a few things (styles, other components, etc.) that we may want to remove. I'll do that in a separate follow-up, to make sure I don't miss any styles. |
|
Flaky tests detected in 4506147. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7637031084
|
|
Gave it a quick test. Didn't spot any issues, and I really enjoyed the improved UX 👍 |
oandregal
force-pushed
the
update/grid-for-patterns-stable
branch
from
f2e5be8 to
4506147
Compare
|
Milestoned this for 17.6, so it's not missed. |
This was referenced Jan 26, 2024
Closed
This was referenced Feb 13, 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.
Part of #55083
What?
This PR makes the dataviews-powered Patterns page stable.
Why?
It's considered to have feature-parity with the existing.
How?
page-patterns/index.jswith those ofpage-patterns/dataviews-patterns.jsand removes the latter.Testing Instructions
Load the "Side Editor > Patterns" page with and without the "admin views" experiment enabled and verify that the dataviews-powered page is loaded.