-
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
PostLockedModal: Update action buttons markup #37837
Conversation
@@ -4,9 +4,6 @@ | |||
|
|||
.editor-post-locked-modal__buttons { | |||
margin-top: $grid-unit-30; |
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 wanted to note that this should be a $grid-unit-15
, and I think we're only using $grid-unit-30
because of margin collapse.
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.
Good thought. I'd love to actually see the more stack-based approach for contents inside here, so we don't have to specify this margin at all, something like this perhaps.
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 agree, or we should switch to top margins for paragraphs.
Size Change: +5 B (0%) Total Size: 1.13 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.
Very nice, works as intended, thank you!
I did notice an issue in testing this, but that's separate and I'll try and look into it:
On a separate note and mostly a general thought on the direction of modals for the project, I'd love to see further componentization and unification. Something like #34153 is good inspiration.
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.
Except for the thing @jasmussen mentions about the responsive view, this looks perfect to me.
Thank you for testing. I see that @jasmussen has already started working on follow-up PR 🦸 |
Description
PR updates Post Locked modal action buttons markup to match other similar models in the core.
Part of #37725.
How has this been tested?
Modal should look the same design-wise.
Create two admin user accounts. Save a post as one user, then log in as the other user in an incognito mode and load the same post.
Screenshots
Types of changes
Code Quality
Checklist:
*.native.js
files for terms that need renaming or removal).