Gallery: Move "Add new" action to Toolbar #25321
Conversation
obenland
added
[Type] Enhancement
|
Next steps:
|
|
Size Change: +763 B (0%) Total Size: 1.18 MB
ℹ️ View Unchanged
|
|
Not sure if the suggested way to do this is the best, any feedback you have I'd love to hear. |
There was a problem hiding this comment.
Tested by test instructions and LGTM!
- Removes the trailing appender
- Introduces an "Add" button with a text label
- "Add" button invokes a dropdown menu with "Open Media Library" and "Upload" options
Naming wise, I would argue that media-add-flow and MediaAddFlow would follow the pattern of media-replace-flow and MediaReplaceFlow. In that sense, it may also best fit under packages/block-editor/src/components/media-add-flow/ dir serving as a component which may be reused from other media galleries perhaps (in the future)?
Should we consider exploring the fact that MediaReplaceFlow may be reused with some minor changes? It already accepts a Name attribute which can be set to Add and it seems we just need to return an array of media instead of a single media item. Apart from that, they seem functioning almost the same (I may be wrong of course). In that case, it could be also renamed to MediaFlow.
<MediaReplaceFlow
mediaId={ ids }
allowedTypes={ ALLOWED_MEDIA_TYPES }
accept="image/*"
name={ __( 'Add' ) }
onSelect={ this.onSelectImages }
onError={ this.onUploadError }
/>
|
I wonder if |
ZebulanStanphill
added
the
Needs Accessibility Feedback
Can't find an evidence of being opinionated, it seems that both add and replace do the same exact thing with different labelling. Not 100% sure though! |
There was a problem hiding this comment.
From an a11y perspective:
- The "Add" button should have an
aria-haspopup="true"to announce it opens an additional UI, like the "More options" button on its right. See also Fix the usage of aria-haspopup #24796 for the general issue. - Since there's a lot of space in the toolbar, can we make the "Add" text more explicit to better differentiate this button from all the other "Add block", "Add this", "Add that" ones?
Changing the text tp "Add image" would be great.
General:
- When opening the media modal, the dropdown menu stays on top of the media modal content (higher z-index I guess), at least for me. Screenshot:
- I see the
input type="file"is hidden withdisplay: none;. Though this relates to the specific component, are we sure it works correctly with all browsers? Recently there has been a similar issue in core where with old Edge and iOS the upload button didn't work. Also, this seems inconsistent with what core does. See https://core.trac.wordpress.org/changeset/47834 and https://core.trac.wordpress.org/ticket/49753 Cc @tellthemachines and @azaozz who know more about this issue.
First pass. This will at least make it functional. Next: Cleanup.
obenland
force-pushed
the
update/gallery-appender
branch
from
0ed3f1d
to
59a4012
Compare
Props @ZebulanStanphill Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com>
|
@afercia Thank you for this excellent review and feedback! I added the a11y improvements you pointed out (I was not aware of The dropdown menu staying on top of the media model is a mystery to me. I have no idea how I missed that during initial development, and I'm not sure how to best go about that. It looks like the z-index values for both components haven't changed in years, so I wonder if there's something that changed with toggling that dropdown open and close? In regards to the |
obenland
added
Needs Technical Feedback
|
@obenland with the merging of the gallery refactor, the Do you want to do that on this PR, or will it be easier to close this and create a new PR for it? |
|
Going to close this for now. The original issue has been added the the gallery refactor project board and we will reimplement this in a new PR against the refactored gallery block. |
Description
Adds a block control to add/upload images to existing galleries.
Removes the dropzone appender at the end of galleries.
Fixes #21247.
How has this been tested?
Screenshots
Types of changes
Checklist: