Skip to content
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

Consolidate edit media icons in toolbar #17220

Closed

Conversation

fabiankaegy
Copy link
Member

Description

Addressed the issue described in #17219 I have added the "new" modify media icon to the Video, Media-Text & Cover block, to make it consistent with the icon used in the image block.

The thing that feels wrong, is that I've added the icon inside each of the block folders, rather that just having it once. If there is a place where these shared resources should go that would make it way cleaner.

How has this been tested?

The included test suite has passed with the changes and the blocks delivered the expected experience in manual testing.

Types of changes

I've changed the edit media icon in the Toolbar of three blocks to make it consistent throughout all blocks.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@fabiankaegy fabiankaegy changed the title Consolidate edit image icon Consolidate edit media icons in toolbar Aug 28, 2019
@talldan talldan added [Block] Cover Affects the Cover Block - used to display content laid over a background image [Block] Media & Text Affects the Media & Text Block [Block] Video Affects the Video Block [Type] Task Issues or PRs that have been broken down into an individual action to take Needs Design Feedback Needs general design feedback. labels Aug 28, 2019
@talldan
Copy link
Contributor

talldan commented Aug 28, 2019

Thanks for working on this @fabiankaegy. There's been some other work underway to change the user flows for media blocks.

#16200 is exploring this for the audio block.

Consistency is always good though. I've added Needs Design Feedback so that the design team can check they're happy with this change.

@fabiankaegy
Copy link
Member Author

Okay awesome, to make it easier to see what Icons I'm talking about, here are two screenshots of the icons in use:

Image Block:

Bildschirmfoto 2019-08-28 um 13 00 27

Cover Block:

Bildschirmfoto 2019-08-28 um 13 00 08

@shaunandrews
Copy link
Contributor

Over in #11952 it looks like we’re planning on replacing the cryptic icon with a label. I’d expect us to do the same for all media related blocks like the ones you show above.

@fabiankaegy
Copy link
Member Author

@shaunandrews Yes and that might be the clearest solution. All I thought is, that the inconsistency is not great.

Also if the icon is going to be used it maybe should be included here so it can be referenced everywhere without being repeated:
Dashicons Gutenberg Icons

@paaljoachim
Copy link
Contributor

I have noticed that the embed Video block, Cover block, Media & Text still uses a pencil and should instead use the same icon as is used in the Image block.

Is the first step to consolidate the edit image/media icons before the first version of this PR is merged #11952 ?

@kjellr @mapk @karmatosed @mtias @draganescu @senadir

@mapk
Copy link
Contributor

mapk commented Sep 9, 2019

@fabiankaegy, thanks for taking the time on this contribution. However, I think we're super close to finalizing the "Replace" media system in #16200. I'd rather extend our efforts there right now. Once this is integrated, we'll need to update all the appropriate blocks and test them extensively.

Can we close this for now? It might be a good idea to line up an issue that identifies all the blocks that will need to be updated once #16200 is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Block] Media & Text Affects the Media & Text Block [Block] Video Affects the Video Block Needs Design Feedback Needs general design feedback. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants