-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Blocks: Add a delete button #1367
Conversation
editor/block-right-menu/index.js
Outdated
className="editor-block-right-menu__control" | ||
onClick={ onDelete } | ||
icon="trash" | ||
label={ __( 'Delete the block' ) } |
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'm sure we'll need some aria roles here that say which block you are deleting :)
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 use the position for the block mover. I can do the same Delete the block at the position %d
but I'm not sure it's relevant since the mover is about swapping positions.
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.
Delete %s block
with the string being the block name maybe?
editor/block-right-menu/index.js
Outdated
*/ | ||
import './style.scss'; | ||
|
||
function BlockRightMenu( { onDelete } ) { |
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.
Can we find a more meaningful name for this one?
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.
For context, it'll contain a delete and a cog button. Not sure how to name it
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.
BlockSettingsMenu 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.
Ok going for that.
I'd like to polish the icon a bit. And perhaps have a smaller version (mockups use 18px gridicons). But that's a separate task. Nice, looks good 👍 |
@jasmussen yep, feel free to update the PR if you want to before or after merge :) |
I think this is a post merge thing ;) Thanks. |
5d737e9
to
e429998
Compare
Let's 🚢 |
closes #130