Problem/Motivation
#2574767-36: Views listing page displays too few items on a page removed the "Tags" column from the list of views at /admin/structure/views
, because it was rarely used.
I don't personally use Views Tags on every project, so I understand with, and largely agree with the reasoning in that issue. Also I couldn't find any other issues asking for it to be re-added, which probably also indicates that the feature is not used that often.
That being said, I do find Views tags very useful in specific situations, e.g.: for managing technical debt on a site with multiple developers and a tight timeline... I tag views by their base table, contextual filters, whether they use aggregation, and where they are used; so that it is easy for developers to find an existing view they can add a new display to (rather than creating an entirely new view), and it is easy to see which views could be combined when they are already a mess.
Ideally, the list of views at /admin/structure/views
would become a view of its own (e.g.: as part of #1823450: [Meta] Convert core listings to Views), which would allow me to simply add a tags column on the sites that required it; and leave it at the default everywhere else.
However, in the meantime, it would be handy to have a patch to apply to add the tags column back.
Proposed resolution
Write a patch to re-add the tags column.
Remaining tasks
Write a patch.
I'll leave it to others to determine whether this is something we want to add back to Drupal core; or just leave here for people to apply when they need it.
User interface changes
Adds a "tags" column between Description and Displays to the list of views at /admin/structure/views
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#63 | after-patch.png | 536.77 KB | smustgrave |
#63 | before-patch.png | 226.54 KB | smustgrave |
#60 | 2929931-after-patch.png | 49.39 KB | Devashish Jangid |
#60 | 2929931-before-patch.png | 49.3 KB | Devashish Jangid |
#59 | reroll_diff_2929931_56-59.txt | 2.44 KB | ankithashetty |
Comments
Comment #2
mparker17Here's a patch; reviews welcome.
At time-of-writing, this patch can be applied to 8.4.x HEAD (commit
6372cc36bc
) and 8.3.x HEAD (commitdda95265df
); it applies cleanly usinggit apply --index
but applies with offset notices inViewListBuilder.php
usingpatch -p1
.Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedThe #2 patch is perfectly applied to the current 8.6.x too. And it works fine (after cleaning the cache). Do we need to add any tests here, or is it just a matter of community approval?
The tags do sometimes help a lot. Especially for filtering a large number of views. See also #2477115: Views UI no longer allows filtering the list by tag, display, or type.
Therefore, deleting tags was probably an extra step in simplification in #2574767: Views listing page displays too few items on a page.
Comment #5
LendudeYes, tests please! why do you think I went through the trouble of adding them? :-) See #2753791: Add javascript testing for the Views listing page.
Should be easy enough to add some assertions to
\Drupal\Tests\views_ui\FunctionalJavascript\ViewsListingTest
Some screenshots of how this will look would be nice and it probably needs a usability review (i think?).
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commented@Lendude, big thanks for hints! Some assertions added.
ViewsListingTest
is a very comfortable place indeed!However, I'm afraid not to collect total approval with one input field for filtering names, descriptions, tags...:
But as a first step may not be so bad? The second step - creating a more complex filtering system in #2477115: Views UI no longer allows filtering the list by tag, display, or type (?)
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #10
Anonymous (not verified) CreditAttribution: Anonymous commentedOops.
Comment #12
sameeran joshi CreditAttribution: sameeran joshi at Google Summer of Code commentedimage after patch
thanku @vaplas but,I am unable to get the "tags" quote in the searchbar. for the patch
2929931-10.patch
Does anyone know about the reason?
Comment #13
sameeran joshi CreditAttribution: sameeran joshi at Google Summer of Code commentedComment #14
Anonymous (not verified) CreditAttribution: Anonymous commented@sameeran joshi, clear cache ;)
Comment #15
sameeran joshi CreditAttribution: sameeran joshi at Google Summer of Code commentedI have cleared it using Administration > Configuration > Development > Performance.
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedAfter applying the patch, I have an absolutely identical screen like you in #12. And using "Clear all cache" (Administration > Configuration > Development > Performance) fixed it.
Another way: go to /update.php and press "Continue" (empty update also triggering clear cache).
Try also remove twig-cache (just remove folder sites/default/files/php/twig)
Try also refresh page via Ctrl+F5 (sometimes Chrome likes to hold a cache)
No other ideas.
Comment #17
LendudeIt would be better to do this in setUp() I think, mixing API calls and functional testing feels a bit wrong.
It also means we don't need the extra drupalGet to reload the page.
Currently we have no assertion that the else actually gets hit. If the setting of 'my first tag' somehow failed (or we take that line out) this would still pass. So maybe also filter on 'my first tag' and make sure we get the right results?
Comment #18
sameeran joshi CreditAttribution: sameeran joshi at Google Summer of Code commentedthanku @vaplas for all suggestions.1,3,4 didn't work well.but could you please elaborate the 2nd one in simple and detailed steps,as i am a newbie to drupal,please
Comment #19
Anonymous (not verified) CreditAttribution: Anonymous commented@Lendude, thanks for these tips! Done + added hook_update.
@sameeran joshi, try using this new patch (#19). Then go to the
/update.php
page and clickContinue
. You should see the following screenshot:Click "Apply pending updates". Should help.
I also want to caution that until the issue has not received approval, so there is a risk that it will be closed. Therefore, while Need review means "to check if it is necessary at all" :)
In fact, I can easily understand that returning a tag as it is can cause a negative reaction among some developers.
But someone uses it. And the tags also allow to remove the default view, a large number of which sometimes interfere with the work (#2945073: Way To Hide system views).
But from the filter through the general input field on several columns it is not enough sense for that :(
I've already looked at a couple of interesting table filters, I'll add information about them to the #2477115: Views UI no longer allows filtering the list by tag, display, or type later.
Comment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedWrong test, sorry!
Comment #22
sameeran joshi CreditAttribution: sameeran joshi at Google Summer of Code commentedit seems that the steps are followed but isn't working well.thanks for support.
Comment #23
dddbbb CreditAttribution: dddbbb as a volunteer commentedI'd love to see this back in core. Top work folks! :D
Comment #25
andypostAlso issue summary needs update with screenshots - before/after to get usability review
It should be post_update hook https://www.drupal.org/node/2960601
Comment #27
LendudeReroll (so no interdiff) and changed update to post update, and added screenshot to the IS.
Comment #29
AdamPS CreditAttribution: AdamPS at AlbanyWeb commentedPatch works for me thanks.
However the issue summary indicates that an earlier commit removed the tags column so it's not necessarily acceptable to put it back. If you don't have a wide screen this patch makes the columns narrow and the page becomes difficult to use.
The ability to filter by tags is useful and has no disadvantage that I can see, so we could likely commit that. We could make the tag column visually hidden by default (CSS?) in a way that would be easy for sites to override. Sites could also choose to hide a different column (most likely machine name) to make room to reveal the tags.
Comment #31
jrearickThis is pretty neat, however, it might not be the greatest to suddenly add it back into the interface. As @AdamPS mentioned, it was removed for a valid reason. Perhaps, in the view settings page, we add a checkbox to show/hide the column with the default off? Adding it to the filter, I think, is fine all the time.
Comment #33
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedPatch #27 can't be applied on 9.2.x .Needs reroll
Comment #34
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedAdded reroll for patch #27.Please check it.
Comment #36
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedUpdated CS and deprecation test failure.
Comment #37
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedComment #38
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commentedI have tested the last patch.The patch adds the Tags column ,but it did not show the view tags.
Comment #39
Bohus UlrychHi all,
very useful functionality for me, I'm voting to have this back.
And I was confused - I can set Administrative tags, but I can't use them anywhere for filtering? Sound like inconsistency.
@anmolgoyal74 Patch #36 is not working - tags are not displayed, because you are adding {{ row.data.tag.data }} in core/modules/views_ui/templates/views-ui-views-listing-table.html.twig, but is should be actually {{ row.data.tags.data }}
It is correct for core/themes/stable/templates/admin/views-ui-views-listing-table.html.twig, but this template is not used in my case.
Thanks
Comment #40
SKAUGHTpatch36 works as expected. patched tested against D8.9.x. using contrib admin theme. simple cache clear.
Comment #42
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Zyxware Technologies commentedVerified and tested patch#36 for drupal 9.3.x-dev version. Patch applied successfully but table columns are misplaced after applying the patch.Adding screenshot for the Reference
Comment #43
xaa CreditAttribution: xaa as a volunteer commentedlooking at Rinku #42 image afterpatch.png it seems there is an issue on the table headings if tags column is empty.
Comment #45
Bohus UlrychFYI patch is not #36 is not fully successfully applied with Drupal 9.3.0 - but problem is only with patching test file core/modules/views_ui/tests/src/FunctionalJavascript/ViewsListingTest.php
I'm still facing issues described in my previous post #39. No one else has same problem?
Comment #46
vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commentedPatch #36 not applying in drupal-9.3.x-dev showing error while going to apply the patch
Needs to re-roll
Comment #47
dilliganesh CreditAttribution: dilliganesh as a volunteer and at UniMity Solutions Pvt Limited for Drupal India Association commented1. I’ve re-rolled the patch #36 and it is working fine in drupal 9.4.x
2. I am able to see the tags with associated values. The TAG column is placed after the column for Description.
3. I added an additional tag for the existing view and it is also getting displayed.
4. Also I am able to filter by Tags.
5. Find below screenshot for the newly applied re-rolled patch
Comment #48
dilliganesh CreditAttribution: dilliganesh as a volunteer and at UniMity Solutions Pvt Limited for Drupal India Association commented1. I’ve re-rolled the patch #36 and it is working fine in drupal 9.4.x
2. I've fixed the custom command failed
Comment #49
dilliganesh CreditAttribution: dilliganesh as a volunteer and at UniMity Solutions Pvt Limited for Drupal India Association commented1. I’ve re-rolled the patch #36 and it is working fine in drupal 9.4.x
2. I've fixed the custom command failed
Comment #50
dilliganesh CreditAttribution: dilliganesh as a volunteer and at UniMity Solutions Pvt Limited for Drupal India Association commented1. I’ve re-rolled the patch #36 and it is working fine in drupal 9.4.x.
2. I am able to see the tags with associated values. The TAG column is placed after the column for Description.
3. I added an additional tag for the existing view and it is also getting displayed.
4. Also I am able to filter by Tags.
5. I've changed the status to 'Needs Review'.
5. Find below screenshot for the newly applied re-rolled patch.
Comment #51
Bohus UlrychGreat job @dilliganesh !
Works like charm - tested with 9.3.0
Comment #52
mkindred CreditAttribution: mkindred commentedI tested 2929931-50.patch against 9.3.2, and it's working for me.
Comment #53
SKAUGHTchanging status as per #51, #52
Comment #55
Chi CreditAttribution: Chi commentedThe patch needs to update Claro's template for the view listing.
core/themes/claro/templates/admin/views-ui-views-listing-table.html.twig
Comment #56
Chi CreditAttribution: Chi commentedComment #57
SKAUGHTThe newest kid/theme on the block has been left out (: thanks
Comment #58
quietone CreditAttribution: quietone at PreviousNext commentedThis is changing the UI so before and after screenshots are needed in the Issue Summary.
Patch does not apply, setting to NW. Un-assigning since others have been working on this for the last two months or so.
These changes appear to be out of scope.
Comment #59
ankithashettyHere is a rerolled patch. Removed changes made to file
core/themes/claro/templates/admin/views-ui-views-listing-table.html.twig
since #3264220: Remove Claro's views-ui-views-listing-table.html.twig template got in. Also addressed the reviews made by @quietone in #58.Thanks!
Comment #60
Devashish Jangid CreditAttribution: Devashish Jangid at Dotsquares Ltd. commentedDrupal 9.4.x-dev
Verified and tested patch #59.
Sharing screenshots for the reference.
Comment #61
Chi CreditAttribution: Chi commentedAnother thing that needs to be fixed is storage schema for the tags. "tag" property actually stores multiple tags. And for that reason should be stored as array not as comma separated strings. This also means it needs to be renamed to "tags". I am not sure if this should belong to this ticket though.
Comment #63
smustgrave CreditAttribution: smustgrave at Mobomo commentedTested #59. Posted screenshots. @chi should there be a follow up ticket for your comment #61
Comment #64
quietone CreditAttribution: quietone at PreviousNext commentedThere are a lot of rerolls and screenshots in the issue when instead what should have happened is action on the Usability review asked for in for in Feb 2018 in #5. This was also pointed out in See #25 with a brief explanation of what needs to be done. And again in #58.
Also, there is a suggestion in #31 that has not been addressed as well as a potential follow up needed for #61 (pointed out in #63).
I also do not see a code review since Feb 2018. Let's get a set of eyes on the code and the test.
A reminder to all to read all the comments and review the issue tags.