-
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
Add template areas to template inspector #35239
Conversation
Size Change: +297 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
@@ -36,9 +37,11 @@ export default function TemplateCard() { | |||
<Icon className="edit-site-template-card__icon" icon={ icon } /> | |||
<div className="edit-site-template-card__content"> | |||
<h2 className="edit-site-template-card__title">{ title }</h2> | |||
<span className="edit-site-template-card__description"> | |||
<div className="edit-site-template-card__description"> |
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.
Any reason why this wasn't a <p>
?
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'd expect it to be a <p>
given description
is escaped and so can't contain markup.
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.
The description in all the blocks are all using <span>
for some reasons. That's why I'm wondering if there are something we haven't thought of?
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.
Weird! No reason it should hold up this PR.
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.
Code looks great and it works when I test locally! Nice one.
); | ||
} | ||
|
||
export default function TemplateAreas() { |
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 see a similar TemplateAreas
component in #35202. Can we re-use?
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.
Probably not a good idea, since that one is using <MenuItem>
and this is using plain <Button>
. I've already moved some common parts to a selector, the rests are mostly UI. We can create a UI component for them if there's a need.
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.
Better to abstract too late than too early!
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.
Approving from the design side. I updated the label to just read "Areas". I figured we didn't need the "Template" prefix.
5368adc
to
cd36f7b
Compare
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.
Description
Addresses a part of #29147.
Add Template Areas to the template inspector.
How has this been tested?
tt1-blocks
themeScreenshots
Kapture.2021-09-30.at.14.10.32.mp4
Types of changes
New feature
Checklist:
*.native.js
files for terms that need renaming or removal).