Problem/Motivation

Inside modules/media/config/optional/views.view.media.yml we have:

        bundle:
          id: bundle
          table: media_field_data
          field: bundle
          relationship: none
          group_type: group
          admin_label: ''
          label: Source

The Source label is wrong, because that column shows Media Types, not source plugins.

Proposed resolution

Use the correct label.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 media_list.png234.86 KBxjm
#3 interdiff-2934649-2-3.txt1.2 KBchr.fritsch
#3 2934649-3.patch2.26 KBchr.fritsch
#2 2934649-2.patch926 bytesmarcoscano
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
CreditAttribution: marcoscano for Acquia commented
Title: Fix wrong header "Source" on media overview page » [PP-1] Fix wrong header "Source" on media overview page
Assigned: marcoscano » Unassigned
Status: Active » Postponed
FileSize
2934649-2.patch926 bytes

Postponing this on #2932369: Media Types missing access control handler result in empty column in media overview page, because we need to update the test that is being added there.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Postponed » Needs review
FileSize
2934649-3.patch2.26 KB
interdiff-2934649-2-3.txt1.2 KB

Unpostponing because #2932369: Media Types missing access control handler result in empty column in media overview page landed, and updated the test.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Title: [PP-1] Fix wrong header "Source" on media overview page » Fix wrong header "Source" on media overview page
marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
CreditAttribution: marcoscano for Acquia commented
Status: Needs review » Needs work 
+++ b/core/modules/media/tests/src/Functional/MediaOverviewPageTest.php
@@ -36,7 +36,7 @@ public function testMediaOverviewPage() {
-    $assert_session->selectExists('source');
+    $assert_session->selectExists('type');

I believe the machine name is still source, so this select wouldn't change its name.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
CreditAttribution: marcoscano for Acquia commented
Status: Needs work » Reviewed & tested by the community

Stupid me, never mind :)

Looks good, as soon as the bot says it's green.

The last submitted patch, 2: 2934649-2.patch, failed testing. View results

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
FileSize
media_list.png234.86 KB

Looks better to me; also seems like a (small) usability improvement. I tested and the changed label totally makes sense in context:

I also manually tested that the renamed "Type" filter behaves as expected.

Committed and pushed to 8.5.x. Thanks!

  • xjm committed ec31a14 on 8.5.x 
    Issue #2934649 by chr.fritsch, marcoscano: Fix wrong header "Source" on...

Status: Fixed » Closed (fixed)

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