-
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
Restore Site Title's Level toolbar and remove obsolete code #24758
Restore Site Title's Level toolbar and remove obsolete code #24758
Conversation
Thanks for digging! It seems like the point of commit 0446ed1#diff-655f76535495d8ae1d82693b57a4ffba (PE #18361) that accidentally recreated cc/ @ellatrix who made the Also cc/ @epiqueras who originally added the Level toolbar in 988ba5d#diff-655f76535495d8ae1d82693b57a4ffba (#20361). Edit: Specifically, I'm curious if we should change the block back to |
const [ title, setTitle ] = useEntityProp( 'root', 'site', 'title' ); | ||
const tagName = level === 0 ? 'p' : `h${ level }`; | ||
const tagName = 0 === level ? 'p' : `h${ level }`; |
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.
We don't enforce Yoda conditions in our JS (since we're enforcing typed comparisons -- ===
instead of ==
-- anyway).
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.
Actually I copy-pasted it from the previous source and yeah, it felt strange to have Yoda conditions in JS. I'll revert this change since I don't like it either.
That sounds like a great idea for a follow-up PR 👍 |
Co-authored-by: Bernie Reiter <ockham@raz.or.at>
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.
Thanks a lot! 🚢
Description
By mistake, we had two edit files. One at
/edit.js
and one at/edit/index.js
. Looks like the Level toolbar functionality was added by this commit 988ba5d#diff-655f76535495d8ae1d82693b57a4ffba at/edit/index.js
and later on/edit.js
was created again by this commit 0446ed1#diff-655f76535495d8ae1d82693b57a4ffba . After this commit, we lost the Level Toolbar which was residing in the/edit/index.js
. Since we had anedit
folder and anedit.js
file, the import was confusing since we had no idea whether the file or the folder being resolved.This PR cleans up the mistake by removing the
/edit.js
and re-adding the Level toolbar.Note:
We have a component at
packages/block-library/src/heading/heading-level-dropdown.js
which seems to be very similar to the one used by Site Title. Probably we should extract the component from the Heading block and reuse it here.How has this been tested?
Screenshots
Types of changes
New feature
Checklist: