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.
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
DataViews: Add duplicate pattern
action in patterns page
#57592
DataViews: Add duplicate pattern
action in patterns page
#57592
Conversation
@@ -44,12 +40,10 @@ export default function DuplicatePatternModal( { | |||
user: getUserPatternCategories(), | |||
}; | |||
} ); | |||
|
|||
if ( ! pattern ) { | |||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a pattern can be undefined at this point? 🤔
Size Change: +516 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing well for me @ntsekouras 👍
I've taken it for a spin with the "new admin views" experiment both enabled and disabled. I didn't spot any issues.
✅ Can create a pattern via the site editor Patterns page's "Create Pattern"
✅ Can import a pattern from JSON on the Patterns page
✅ Can create a pattern from selected blocks in the post editor
✅ Can duplicate patterns via the dataviews duplicate action
✅ Can duplicate a pattern while editing it via the command palette
I think the duplicated CSS for the modal is ok until we consolidate it all when the experiment lands. If the TODO
comment is updated to reflect that, this should be fine to land.
// TODO: this is duplicated from `patterns-menu-items__convert-modal` styles, | ||
// except for the `z-index`. Need to check if this is still needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the CSS for the modal was required to both constrain the width of the modal and prevent a subtle layout shift when interacting with the category selection control. The rest I think was to bring the styling of the category selector up to the latest approach.
Quickly disabling those styles and interacting with the modal resulted in this:
id: 'duplicate-pattern', | ||
label: __( 'Duplicate' ), | ||
isEligible: ( item ) => item.type !== TEMPLATE_PART_POST_TYPE, | ||
modalHeader: __( 'Duplicate pattern' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found these strings ambiguous, so I proposed _x
in #57686
|
||
// TODO: this is duplicated from `patterns-menu-items__convert-modal` styles, | ||
// except for the `z-index`. Need to check if this is still needed. | ||
.dataviews-action-modal__duplicate-pattern { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntsekouras @aaronrobertshaw can we do this differently, without using an internal dataviews class? I worry that by doing this kind of thing, the dataviews internal classes become public API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same answer as here:
Personally I don't think it's bad to have a generated class name based on the unique action's id. I'm also not too concerned about indicating a public API as we can have breaking changes in Data Views.
Having said that, the alternative would be to have
modalProps
in actions API and that would also indicate public API. I think we'll know more when we are preparing for extending the APIs and update as needed then.
What?
This PR adds the
duplicate pattern
action in the patterns page. I haven't yet handled the duplication of template parts to reduce the scope.Duplicating a pattern and creating a new one right now utilize many shared components like
DuplicatePatternModal
andCreatePatternModal
. Since in DataViews' actions the modal is handled internally, I moved around some pieces of code and added some extra private APIs. Maybe simplifications could be made for components to do less - I think right now they are doing more than they should, but that's something to explore separately.Notes
I added some extra things in the DataViews actions:
modalHeader
property to override if needed.Testing Instructions
Duplicate
action that works properlyCreatePatternModal
andDuplicatePatternModal
are usedScreenshots or screencast
Screen.Recording.2024-01-05.at.3.33.12.PM.mov