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

Gallery Block: adding buttons to remove image from galleries inline #2658

Merged
merged 2 commits into from
Sep 5, 2017

Conversation

youknowriad
Copy link
Contributor

screen shot 2017-09-04 at 12 14 10

fixes #2128

@youknowriad youknowriad added [Feature] Blocks Overall functionality of blocks Needs Design Feedback Needs general design feedback. labels Sep 4, 2017
@youknowriad youknowriad self-assigned this Sep 4, 2017
@mtias
Copy link
Member

mtias commented Sep 4, 2017

I think you should "select" an image before being able to remove it. That should help with UI noise as well. (I explored it a bit in an old captions PR.)

@codecov
Copy link

codecov bot commented Sep 4, 2017

Codecov Report

Merging #2658 into master will decrease coverage by 0.07%.
The diff coverage is 14.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2658      +/-   ##
==========================================
- Coverage   31.38%   31.31%   -0.08%     
==========================================
  Files         177      178       +1     
  Lines        5413     5429      +16     
  Branches      949      949              
==========================================
+ Hits         1699     1700       +1     
- Misses       3139     3154      +15     
  Partials      575      575
Impacted Files Coverage Δ
blocks/library/gallery/index.js 62.5% <ø> (+29.16%) ⬆️
blocks/library/gallery/gallery-image.js 55.55% <100%> (+5.55%) ⬆️
blocks/library/gallery/block.js 11.76% <11.76%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f445861...fe7dd08. Read the comment docs.

@youknowriad
Copy link
Contributor Author

I think you should "select" an image before being able to remove it

how should we "show" the selected state? A stronger border on the image? any design proposals?

@mtias
Copy link
Member

mtias commented Sep 4, 2017

This was Joen's proposal back then:

image

@mtias
Copy link
Member

mtias commented Sep 4, 2017

For reference: #1831

@munirkamal
Copy link
Contributor

This looks great addition to the gallery block. Also how about adding the possibility to re-arrange the images in a gallery block inline, by simply drag-n-drop?

May be extending the "Joen's proposal" and add another icon for "move" which would allow to hold that image and move it within gallery block to drop on the desired place.

@youknowriad
Copy link
Contributor Author

@mtias this toolbar in this proposal could overlap the block's controls toolbar. I'm putting it under the image to start.

@youknowriad
Copy link
Contributor Author

After several attempts, I think showing the menu inside the image is the best approach for now. We'll have to revisit the "overflow: hidden" style of the wrapper if we want something like the proposal (above or under the image)

screen shot 2017-09-04 at 15 26 37

@youknowriad youknowriad force-pushed the add/remove-image-from-gallery branch from 2b73158 to 127c764 Compare September 4, 2017 14:28
@youknowriad
Copy link
Contributor Author

@munirkamal there's an issue for that too #743 ;)

Feel free to add any complementary proposal/idea on the issue.

Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Sep 4, 2017
@mtias
Copy link
Member

mtias commented Sep 4, 2017

@youknowriad I think the X was better in this case, as the trash icon can sort of imply "deleting the image".

@karmatosed
Copy link
Member

On seeing your comment @mtias I took a step outside and saw what existing pattern there was on in WP.

2017-09-04 at 21 24

In actual images:

2017-09-04 at 21 25

It made me think how obvious hovering is to seeing deleting. Would it maybe be good to see like this? Perhaps that's something to investigate though in testing.

@youknowriad youknowriad force-pushed the add/remove-image-from-gallery branch from 127c764 to fe7dd08 Compare September 5, 2017 07:52
@youknowriad youknowriad merged commit da364e8 into master Sep 5, 2017
@youknowriad youknowriad deleted the add/remove-image-from-gallery branch September 5, 2017 07:58
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gallery: Add inline editing to allow removing images from block
4 participants