Problem/Motivation

The Add or Select Media dialog is inconsistent about use of singular and plural, which could cause editor confusion. It would be better to be consistent.

Steps to reproduce

1. Create a Drupal Core site in simplytest.me
2. Log in as admin
3. Go to /admin/modules
4. Filter on media
5. Enable Media and Media Library
6. Click Install
7. Upon completion, go to /admin/config/content/formats/manage/basic_html
8. Drag the picture icon out of the active area
9. Drag the media icon into the active area
10. Under Enabled Filters, check Embed media
11. Under Filter Settings, Embed media, check all boxes
12. Click Save configuration
13. Go to /node/add/page
14. Click the Insert media icon

Expected result:
Add file
Choose file
no file selected
One file only

Actual result:
Add file
Choose files
no files selected
One file only

Screen capture of Add or select media dialog, showing actual result as stated

Proposed resolution

Change above plurals to singular

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#15 Screenshot 2024-03-21 at 12.29.46 PM.png228.44 KBsukr_s
#6 3418781-nr-bot.txt1.22 KBneeds-review-queue-bot
#4 7015.patch1.33 KBsukr_s
Add or select media dialog.png161.9 KBcharles belov

Issue fork drupal-3418781

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Charles Belov created an issue. See original summary.

sukr_s made their first commit to this issue’s fork.

sukr_s opened merge request !7015

sukr_s’s picture

sukr_s commented
Status: Active » Needs review
StatusFileSize
new 7015.patch1.33 KB

patch enclosed. #multiple is set based on the number of files upload allowed. this sets the ui text properly

dineshkumarbollu’s picture

dineshkumarbollu commented

@sukr_s

MR fails pipeline due to phpcs, can you fix that i can review the MR, Thanks

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3418781-nr-bot.txt1.22 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

sukr_s’s picture

sukr_s commented
Status: Needs work » Needs review

PHPCBF issue fixed

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs tests

As a bug will need a failing test to show the issue.

sukr_s’s picture

sukr_s commented
Status: Needs work » Needs review

@smustgrave The fix resolves correctness of English in UI (plural vs. singular). Not sure how to add tests for this. If you can point to some documentation, that would be helpful.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

MR has a test failure

UI changes should also have before/after screenshots in the issue summary.

Tests can be a simple assertion added to an existing test.

sukr_s’s picture

sukr_s commented
Status: Needs work » Needs review

MR updated with fix and test cases have been updated as well.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left a comment.

sukr_s’s picture

sukr_s commented
Status: Needs work » Needs review

.

sukr_s’s picture

sukr_s commented
Assigned: Unassigned » sukr_s
Status: Needs review » Needs work
sukr_s’s picture

sukr_s commented
Assigned: sukr_s » Unassigned
Status: Needs work » Needs review
StatusFileSize
new Screenshot 2024-03-21 at 12.29.46 PM.png228.44 KB

The button label has to change to "Choose file". I believe there is no way to assert this (or?). Attach screenshot from the automated run that shows that the label has been successfully changed

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +Needs Review Queue Initiative

Yea in that case I think the negative assertions should be fine.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

One question on the MR.

sukr_s’s picture

sukr_s commented
Status: Needs work » Reviewed & tested by the community

The original code was always for "multiple files" and the test cases as well. The UI showed upload "files" even when a single file was being uploaded. So the fix was for ensuring that the right text is shown in the UI in case of single file upload. No other functionality or test cases are affected. e.g. line 300

 // Assert we can add multiple files.
 $this->assertTrue($assert_session->fieldExists('Add files')->hasAttribute('multiple'));

tests for multiple files

  • catch committed d0469464 on 10.3.x 
    Issue #3418781 by sukr_s, Charles Belov, smustgrave: UI refers to "files...

  • catch committed f4bf71d6 on 11.x 
    Issue #3418781 by sukr_s, Charles Belov, smustgrave: UI refers to "files...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Ahh yeah, bad ctrl-f on my part it looks like, I can see coverage for both 'Add file' and 'Add files' now.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.