Skip to content

Blocks: Add a delete button#1367

Merged
youknowriad merged 2 commits intomasterfrom
add/block-delete-button
Jun 22, 2017
Merged

Blocks: Add a delete button#1367
youknowriad merged 2 commits intomasterfrom
add/block-delete-button

Conversation

@youknowriad
Copy link
Copy Markdown
Contributor

closes #130

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Jun 22, 2017
@youknowriad youknowriad self-assigned this Jun 22, 2017
@youknowriad youknowriad requested review from jasmussen and mtias June 22, 2017 12:52
@youknowriad youknowriad added [Feature] Blocks Overall functionality of blocks [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels Jun 22, 2017
className="editor-block-right-menu__control"
onClick={ onDelete }
icon="trash"
label={ __( 'Delete the block' ) }
Copy link
Copy Markdown
Member

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 :)

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Member

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?

*/
import './style.scss';

function BlockRightMenu( { onDelete } ) {
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BlockSettingsMenu perhaps?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok going for that.

@jasmussen
Copy link
Copy Markdown
Contributor

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 👍

@youknowriad
Copy link
Copy Markdown
Contributor Author

@jasmussen yep, feel free to update the PR if you want to before or after merge :)

@jasmussen
Copy link
Copy Markdown
Contributor

I think this is a post merge thing ;) Thanks.

@youknowriad youknowriad force-pushed the add/block-delete-button branch from 5d737e9 to e429998 Compare June 22, 2017 13:21
@mtias
Copy link
Copy Markdown
Member

mtias commented Jun 22, 2017

Let's 🚢

@youknowriad youknowriad merged commit d79bc78 into master Jun 22, 2017
@youknowriad youknowriad deleted the add/block-delete-button branch June 22, 2017 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Blocks Overall functionality of blocks [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add delete button

3 participants