Opened 6 years ago
Closed 6 years ago
#41994 closed defect (bug) (fixed)
New Gallery Widget: Add Media button label should be updated for consistency
Reported by: | juhise | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Widgets | Keywords: | has-screenshots good-first-bug has-patch |
Focuses: | ui | Cc: |
Description
I noticed in previous Media widgets Button: label is Add Image
, Add Video
but in gallery widget Select Images
is being used. I think it should be Add Images
Attachments (10)
Change History (39)
#1
@
6 years ago
- Keywords has-patch has-screenshots added
- Milestone changed from Awaiting Review to 4.9
#2
@
6 years ago
I just had a look again and I felt Add Gallery
would look better and when no media is selected label should be No gallery selected
#3
follow-up:
↓ 5
@
6 years ago
"Add Gallery / No gallery selected" feels a little bit like it implies galleries are a unit of images you can select, so I'm personally leaning more towards "Add images / No images selected" as a better solution.
This ticket was mentioned in Slack in #core-customize by melchoyce. View the logs.
6 years ago
#5
in reply to:
↑ 3
@
6 years ago
Replying to melchoyce:
"Add Gallery / No gallery selected" feels a little bit like it implies galleries are a unit of images you can select, so I'm personally leaning more towards "Add images / No images selected" as a better solution.
I love this improvement, but had the same reaction as Mel and would also prefer the "Add images / No images selected".
#6
@
6 years ago
@melchoyce @joemcgill
We are going to add unit of images (Gallery) only and everywhere else we have Edit Gallery
, Replace Gallery
.
#7
@
6 years ago
"Edit Gallery" makes sense to me, because at this point you have created a gallery as a unit of content. You're then editing the contents of the gallery, rather than editing the images themselves, and replacing the gallery with an entirely new set of images, rather than replacing individual images. (Though, I wonder if "Replace" is even necessary, since "Edit" is functionally the same in this particular case?)
#8
@
6 years ago
Yeah agree Edit Gallery
make sense to me too. Replace gallery gives the option to create a new gallery and add it to widget. Thought not sure what would be the real case scenario that anyone would need to create new gallery rather then editing the existing one.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-customize by melchoyce. View the logs.
6 years ago
#11
@
6 years ago
Chatted with @westonruter and our final decision is:
- [ Add Images ] before picking images
- [ Edit Gallery ] after picking images
- Remove [ Replace Gallery ]
@juhise Do you want to make another patch?
#13
follow-up:
↓ 14
@
6 years ago
@melchoyce
That sounds great, we already have patch for Add Images
. I don't think I would like to add anything else.
#14
in reply to:
↑ 13
@
6 years ago
- Keywords good-first-bug needs-patch added; has-patch removed
Replying to juhise:
@melchoyce
That sounds great, we already have patch for
Add Images
. I don't think I would like to add anything else.
Okay, thank you for your contributions :)
Would anyone else like to pick this up and work from @juhise's patch? Would love to merge it tomorrow.
This ticket was mentioned in Slack in #core by melchoyce. View the logs.
6 years ago
#16
@
6 years ago
@melchoyce @westonruter I'd love to attach a patch, but I'm not sure how to "Remove [ Replace Gallery ]". This button gets added in class-wp-widget-media.php for any media widget.
#17
@
6 years ago
@Presskopp Please see the strings defined in the gallery subclass: https://github.com/WordPress/wordpress-develop/blob/1eba2e5/src/wp-includes/widgets/class-wp-widget-media-gallery.php#L31-L34
#18
@
6 years ago
Thanks, yes, I see the strings there, but obviously I still don't get the point. What is the desired result regarding the 'replace' button ?
Attached a patch, though, let me know if there's more to change (or just do it yourself)
#19
@
6 years ago
I think the idea is to just remove the replace button, attached 41994.4.diff for the sake of time :)
#22
follow-up:
↓ 25
@
6 years ago
@helen @helen That removes the Replace button from all media widgets, so we can't do that. It would mean you could never change the an image/video/audio after you selected it the first time, right?
#23
@
6 years ago
I think a better approach would be to just hide the .change-media
button in the Gallery widget via CSS. Or we could wrap the button in the template with a conditional:
<# if ( ! data.hideReplaceButton ) { #> <button type="button" class="button change-media select-media selected"> <?php echo esc_html( $this->l10n['replace_media'] ); ?> </button> <# } #>
And then introduce this hideReplaceButton
prop in the Gallery widget only.
#24
@
6 years ago
Or, alternatively, this condition could be based on whether or not $this->l10n['replace_media']
is populated to begin with in PHP. So then WP_Media_Widget_Gallery
could define replace_media
as null
and then the template could do:
<?php if ( ! empty( $this->l10n['replace_media'] ) ) : ?> <button type="button" class="button change-media select-media selected"> <?php echo esc_html( $this->l10n['replace_media'] ); ?> </button> <?php endif; ?>
#25
in reply to:
↑ 22
@
6 years ago
Replying to westonruter:
@helen That removes the Replace button from all media widgets, so we can't do that. It would mean you could never change the an image/video/audio after you selected it the first time, right?
Ah yes, I forgot about the separate edit details part.
Replying to westonruter:
Or, alternatively, this condition could be based on whether or not
$this->l10n['replace_media']
is populated to begin with in PHP.
Should it check in PHP or in the JS template? I guess that's a matter of semantics and maybe ever so slightly performance?
#27
@
6 years ago
@westonruter patch attached like proposed in https://core.trac.wordpress.org/ticket/41994#comment:24
Patch file