-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 template pattern action #57638
Conversation
Size Change: +147 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
@@ -283,3 +281,11 @@ | |||
max-height: $grid-unit-60 * 2; // Adjust to not cover the save button, showing three items. | |||
} | |||
} | |||
|
|||
.dataviews-action-modal__duplicate-template-part { |
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 didn't see where this class is added. I'm guessing it's generated. I wonder if we should rely on a generated class. I feel the "width" should be a prop of the modal and maybe a config in the action but that's for later.
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.
Yeah, it is generated and for now I wanted to avoid extra modal props in action. We can revisit soon with new actions 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.
@ntsekouras @youknowriad 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.
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 template action in the patterns page
Testing Instructions
CreateTemplatePartModal
is used