Problem/Motivation

Too many options under configuration options for fields/ filters/contextual filters/ relationships: Participants feel “overwhelmed” when they see the long scrolling list of options. The problem gets worse because the participants do not necessarily see the “Search” option. This has multiple causes:

  • The description text makes it significantly harder to scan, this because it visually creates two lines and creates highly variable rows.
  • The description text often adds no additional information.
  • The design has insufficient spacing, also decreasing readability - because its not a standard table design.
  • The filter/search is awkwardly placed, causing them to be often missed.

The criticality of this issue is determined by the fact that this completely overwhelms users, to the point that they either start pogo-sticking or looking elsewhere. The overwhelming effect of Views has many parts, this is one of the very important contributors.

Proposed resolution

The solution is relatively meta, its attacking this problem on multiple axes. The most important issue, that this issue intends to solve is the fact that by not using tables and grouping information we are largely creating that overwhelming effect.

The main proposal is to turn it into a table and group the information:

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Manually test the patch Novice Instructions
Embed before and after screenshots in the issue summary *for current 8.0.x head* and *for the most recent patch* Novice Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

User interface changes

  • Changes div listing into table.
  • Moves the search box into the top

before

after patch #91

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because usability issues in a core task of the views ui
Issue priority Major because user testing shows users have trouble completing add field task
Prioritized changes The main goal of this issue is usability
Files: 
CommentFileSizeAuthor
#128 core-js-views-ui-select-1832862-127.patch8.63 KBnod_
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,000 pass(es). View
#128 interdiff.txt2.32 KBnod_
#127 interdiff.txt5.86 KBnod_
#127 core-js-views-ui-select-1832862-126.patch8.09 KBnod_
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,981 pass(es). View
#119 core-users_feel_overwhelmed-1832862-119.patch7.62 KBLendude
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,483 pass(es). View
#119 interdiff-1832862-112-119.txt3.64 KBLendude
#112 109-112_interdiff.txt494 bytesZekvyrin
#112 core-users_feel_overwhelmed-1832862-112.patch7.76 KBZekvyrin
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,512 pass(es). View
#110 109-110-interdiff.txt1.53 KBMaouna
#110 core-users_feel_overwhelmed-1832862-110.patch8.61 KBMaouna
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,333 pass(es). View
#93 after-field-add.png268.99 KBYesCT
#93 before-add-field.png290.56 KBYesCT

Comments

Bojhan’s picture

Issue summary: View changes

lalalala

yoroy’s picture

I did a quick review of the current labels and descriptions. It's pretty clear we won't get away with not having any descriptions for most of the items. So the idea of having a table for this seems good. I wonder if 'Category shouldn't be the last column, as there will be a lot of redundant 'Content' cells. But that's a detail.

lisarex’s picture

+1 to making column the last column. It's by far the least useful.

tim.plunkett’s picture

@lisarex, er, which column is that? :)
I assume you meant category.

tim.plunkett’s picture

Status: Active » Needs review
FileSize
3.66 KB
FAILED: [[SimpleTest]]: [MySQL] 48,704 pass(es), 63 fail(s), and 0 exception(s). View

Well, this won't work 100% right now because the filters rely on #states, which don't work on table rows.

Status: Needs review » Needs work

The last submitted patch, vdc-1832862-4.patch, failed testing.

dawehner’s picture

It could be that #991454: Add element #type table (with tableselect and tabledrag support) would give us the feature automatically to use #states for tables.

drupalshrek’s picture

When I look at the proposed solution picture I see the filter set to "All" but the table showing only Category content fields. If the filter is set to all, shouldn't we see all the fields? Or do I misunderstand how the proposal is going to work?

lisarex’s picture

@drupalshrek the vast majority are in the Content category and Content items are listed first by default (as the categories are sorted alphabetically) so I can see how that would be confusing, but that's how it would look :)

xjm’s picture

Yeah, the screenshot is just a mockup, not actual output. If "All" were selected, the table would continue to scroll below that, listing many other options

drupalshrek’s picture

OK, fair enough, but I would suggest that the mockup look correct, i.e. either show "Content" in the filter, or show entries from multiple categories and scroll bars.

tim.plunkett’s picture

The mockup is not canonical. The implementation will be correct, it will not be skewed by a reasonably accurate mockup.

xjm’s picture

And we'll add a screenshot of the real thing once the real thing exists. ;)

xjm’s picture

FileSize
35.46 KB

@drupalshrek, here you go.

add-fields-modal-2.png

dawehner’s picture

FileSize
4.05 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1832862-14.patch. Unable to apply patch. See the log in the details link for more information. View

So for a short momenent I thought it could work out, to use '#type' => 'tableselect' but the result wasn't really 100% successfull, so look at this patch just as an academical research :)

sun’s picture

I do not understand why the proposal is to make the category column last, or why you believe that it is least important.

When I look at the screenshots of the proposed solution, then I see this:

Thing | Context | Yada
Thing | Context | Yada
Thing | Context | Yada
Thing | Context | Yada
Thing | Context | Yada

Whereas:

  1. Thing is not unique.
  2. Thing depends on Context to make sense.
  3. Thing is only clearly identifiable if the entire table is filtered by a specific Context.
  4. Otherwise, there can be multiple Things with the same name, but which belong to different Contexts.

Consequently, the "Category" context is the most important data that is presented to me here:

Category | Title | Description

dawehner’s picture

Thing depends on Context to make sense.

To be concrete I think an example could be "Node: Edit link" and "User: Edit link" so we have the same title in both contexts.

The question though is, whether the context is really the major point here.
Just think about the users, which just want to configure a simple view. They probably just know that they want to display the title, so the
maybe not look for content and then for title, but sure, talking about how a user things without real data is often pointless.

yoroy’s picture

I agree about the yada yada parts :-)

But name or title always comes first. The screenshot in #13 shows why the Category isn't that helpful: most of the items in the list are of the same category, so it doesn't really help you tell one from the other. Also, most rows would start with the same word, making it more work to tell items apart from each other.

I'm happy to break the consistency rule when useful, but this is not a place to do that.

Would be good to move this forward, it would really clean up some critical screens in the views UX and #1875252: [META] Make the block plugin UI shippable might benefit from this as well.

hass’s picture

Re 16#: These bad strings no longer exists in D8. See #1779658: Translatable strings not self-explanatory / not context-independent.

Bojhan’s picture

I agree with @yoroy on this thing here, although @sun is right that Context is one of the most important things, its not the primary differentiator which is the title. Lets see if we can make them close.

I'd like to get this issue moving though, this is such a major improvement already. Could this get a working roll? I'd be great to get this in.

yoroy’s picture

So, next patch should build on the one in #4?

jerdavis’s picture

Status: Needs work » Needs review
FileSize
4.12 KB
FAILED: [[SimpleTest]]: [MySQL] 57,268 pass(es), 139 fail(s), and 0 exception(s). View

Reroll of #4 against head

jerdavis’s picture

Status: Needs review » Needs work

Noticing that the filter drop down isn't triggering a change. Running out of time to look at it tonight, but the patch should at least apply for further work.

jerdavis’s picture

FileSize
4.52 KB
FAILED: [[SimpleTest]]: [MySQL] 55,663 pass(es), 139 fail(s), and 0 exception(s). View

Started on another option for rendering this that may work better with #states, if I or someone else can sort it out.

hass’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, vdc-1832862-23-alternative.patch, failed testing.

Bojhan’s picture

Issue tags: +sprint
jerdavis’s picture

FileSize
4.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch vdc-1832862-27.patch. Unable to apply patch. See the log in the details link for more information. View

This patch is a bit more refined. Switched the table to use the #table Form API type and tableselect pattern. This is working for adding items, and the item search works, however #states is still not getting processed on the table row. I'm having trouble tracking down exactly why as I'm not as familiar with some of this stuff.

yoroy’s picture

Status: Needs work » Needs review

triggering testbot

dawehner’s picture

Yeah that has been the points we identified before. Maybe putting in a screenshot of the current status would be cool.

jerdavis’s picture

FileSize
150.94 KB

Here's a screenshot.

views-1832862.png

jerdavis’s picture

FileSize
23.1 KB

I do think there is potentially a usability concern with the column arrangement and the removal of the context from the grouping, take this for example:

views-1832862-columns.png

Is the context of which one you want to choose too far away? I could see how this might be confusing.

dawehner’s picture

If we would rearrange the category to be the first one, we would not run into that problem.

jerdavis’s picture

I understand why we don't want to have the category be the first column, I think the points mentioned on this above make sense. But I'd advocate moving it to the second column potentially, or injecting the category below the title, or in the description string.

xjm’s picture

Yeah, #33 makes sense. So back to Title | Category | Description ?

yoroy’s picture

But how often will this happen? I'm still worried this is optimizing for relative edge cases. How much of a problem is it that in those cases you have to rely on the information in the last column? Also, most of the screenshots I've seen so far have 'Content' as the category, which would mean promoting a column with very repetitive content over more specific descriptions.

To me, the screenshot in #31 shows a problem with the labeling for 'Title' itself, (and/or with the description), for which reordering the columns would not be not the right fix.

jerdavis’s picture

Re-roll to make group second column.

jerdavis’s picture

FileSize
4.3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch vdc-1832862-35.patch. Unable to apply patch. See the log in the details link for more information. View

Seem to have lost the file somewhere over Montana. Let's try that again.

jerdavis’s picture

@yoroy I do agree, particularly with the sentiment that the problem is the label of title, rather than the placement of the group column. Would it be too repetitive to include the group information as subtext for the title?

jerdavis’s picture

FileSize
23.13 KB

What about something like this

Views Add Item table

yoroy’s picture

Yes that'd be too much because with that, we'd be moving back to the current design, which is the *problem* :-)

jerdavis’s picture

Fair enough, leading with the title I think helps a bit but you're right it's pretty close.

tstoeckler’s picture

The duplicity problem could also be fixed by simply re-wording the description in the "Content revision" group to read "The title of the content revision." or something. A duplicate description is pretty strange to begin with.

dawehner’s picture

@tstoeckler
I'm not sure whether this helps if you have several fields on different entity types with relationships connected.

tstoeckler’s picture

Well, then I think the Relationship label should be displayed, IMO. The purpose of descriptions is to make things clear in case the title isn't clear enough on its own. That is completely lost here, though. In that specific case the descriptions make the interface *more* confusing because they distract from the fact that two different things are in fact different.

Just my two cents, though, carry on. :-)

dawehner’s picture

So, you would suggest to show the same fields multiple times? That's certainly out of scope of this issue, but interesting to discuss in another issue.

dawehner’s picture

Issue tags: +Needs screenshots

It would be cool to have before/after screenshots of the current approach.

klonos’s picture

I tried the patch from #37 over simplytest.me in order to get before/after screenies, but both installations with and without the patch appear the same. Was something committed in the meantime?

klonos’s picture

#27: vdc-1832862-27.patch queued for re-testing.

Trying the patch gave a "An error occurred while patching the project." error. Let's see what the testbot thinks...

klonos’s picture

#37: vdc-1832862-35.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Usability, +Needs screenshots, +sprint, +VDC, +BADCamp2012UX

The last submitted patch, vdc-1832862-35.patch, failed testing.

Bojhan’s picture

I dont know why we are trying to solve this the hard way, I still think that the proposal in the issue opener works fine. We should not go for any solution that requires two lines, and I honestly think we are overdoing the optimisation a little

Bojhan’s picture

Issue summary: View changes

yes

klonos’s picture

Issue summary: View changes

...child issues moved to the new issue relations metadata section (actually this issue was set as a parent of both because we currently cannot add children/follow-ups from the parent issue node: #2130889: Allow adding child/follow-up issues directly from the parent issue and converting related issues to children.)

Bojhan’s picture

So we discussed this at Vienna, and we concluded to continue with the proposed solution. All other solutions require two lines and with that make it harder to scan.

We do wish to change the label of Filter to Category.

Bojhan’s picture

Assigned: Unassigned » dawehner
royal121’s picture

FileSize
4.31 KB
PASSED: [[SimpleTest]]: [MySQL] 59,413 pass(es). View

Here is a new patch!

royal121’s picture

FileSize
4.31 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1832862-56.patch. Unable to apply patch. See the log in the details link for more information. View

That last one applies but doesn't work. Hope this on works well.

royal121’s picture

Status: Needs work » Needs review
royal121’s picture

Issue tags: -Needs screenshots
FileSize
36.83 KB
45.91 KB

Here is the screenshot before applying the latest patch:
Before patch

Here is the screenshot after applying the patch:
After patch

Bojhan’s picture

@royal Thanks for this update, I think category was supposed to be next to the title column though.

odegard’s picture

In addition to type "- All -", would it make sense to add another category "- Popular -" or "- Commonly used -" where we put the most used fields?

It would help new users building simple views, and it might also help more advanced users to save clicking and scrolling. Even advanced views may contain the "popular" fields.

I would also suggest we have a default value different than "- All -". On some larger sites I work on just opening the add field dialog takes many seconds, especially since the content is loaded before the window is resized and content is rerendered. Having a smaller set in the default listing would make things much faster.

xjm’s picture

56: 1832862-56.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 56: 1832862-56.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
4.22 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View

Here is a reroll of that with a couple of minor changes thrown in. I guess the main thing we need to work out is the usage of states in the table rows.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This seems to be certainly better than before

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 63: 1832862-63.patch, failed testing.

yoroy’s picture

We were so close! :-)

dawehner’s picture

Issue summary: View changes
Issue tags: +Needs reroll, +Novice

So let's reroll it first.

tadityar’s picture

Status: Needs work » Needs review
FileSize
4.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,572 pass(es). View

Re-rolled. Wow this is from 2 years ago..

eidoscom’s picture

Status: Needs review » Reviewed & tested by the community

It seems ok!

tim.plunkett’s picture

Assigned: dawehner » Unassigned
Status: Reviewed & tested by the community » Needs review
Issue tags: -Needs reroll +Needs screenshots

Can we get some updated screenshots first? Thanks!

tadityar’s picture

Before-After screenshot
Before :
before-patch

After :) :
after-patch

tim.plunkett’s picture

Status: Needs review » Needs work
Issue tags: -Needs screenshots
  1. +++ b/core/modules/views_ui/src/Form/Ajax/AddHandler.php
    @@ -104,8 +104,15 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    +          t('Title'),
    +          t('Description'),
    +          t('Category'),
    

    This can be $this->t().

  2. +++ b/core/modules/views_ui/src/Form/Ajax/AddHandler.php
    @@ -104,8 +104,15 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    -        '#default_value' => 'all',
    +
           );
    

    Random new line

Finally, I manually tested the "Type" filter, and it is broken now.

tim.plunkett’s picture

Issue tags: +Needs manual testing
yoroy’s picture

And per #33, #34 and #59 we'd still like to order the columns as Title | Category | Description

Lendude’s picture

Issue in #72 is due to Types selection using #states and the layout now uses #type => 'table', which doesn't seem to support #states on a per-row basis.
Tried using #type => 'tableselect', but no joy there either for #states per row.

Jeff Burnz queued 68: 1832862-68.patch for re-testing.

nlisgo’s picture

Status: Needs work » Needs review
FileSize
4.27 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,932 pass(es). View
2.07 KB

I have addressed some of the issues in this patch.

I have addressed points 1 and 2 in #72 and issue in #74.

Remaining issue is to get the 'Type' filter working again see comments #72 and #75.

nlisgo’s picture

FileSize
4.15 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Setup environment: Test cancelled by admin prior to completion. View
1.51 KB

This fixes an issue with the searchText and makes the targeting of the label and description a little less fragile.

'Type' filter is still broken.

I left in some debugging code by mistake. I will resupply a patch.

nlisgo’s picture

FileSize
4.06 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,928 pass(es). View
631 bytes

Removed the console.log

The last submitted patch, 78: users_feel_overwhelmed-1832862-78.patch, failed testing.

nlisgo’s picture

Status: Needs review » Needs work

Needs work.

Type filter is broken since the switch to a table. I need to step away from this for now. Might get around to it early next week if there is no activity before then.

dajjen’s picture

The patch #79 no longer apply using latest D8. I'm looking in to it maybe tomorrow.

nlisgo’s picture

Status: Needs work » Needs review
FileSize
4.05 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,435 pass(es). View

Re-roll. I will switch back to 'Needs work' regardless of the outcome of the test run. I just want to trigger the testbot.

dajjen’s picture

What about making the view dialog window a little more wide (70%) and also make the list a little more compact?
screenshot

disasm’s picture

Issue tags: +Needs reroll

This needs a reroll again.

metzlerd’s picture

Title: Users feel overwhelmed by handler listings » Make views add field scannable
Issue summary: View changes
Issue tags: +Triaged at DrupalCon Los Angeles 2015

Issue was understandable, but needed retitling to better reflect problem being solved.
Summary was up to date,
It was reproducible.
No duplicates found.
Priority is appropriate.
Added Beta evaluation
.

B_man’s picture

Status: Needs review » Needs work

Based on comments #85 and #86 this issue needs a reroll, I am going to attempt that now.

B_man’s picture

Status: Needs work » Needs review
FileSize
4.05 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,355 pass(es). View

I was able to get this to apply cleanly (i think).

tim.plunkett’s picture

Issue tags: -Needs reroll

Thanks for the reroll @B_man!

Bojhan’s picture

Thanks!

The title column should be bold. The width can be optimised, but lets leave that for a followup where we can explore some more.

ohthehugemanatee’s picture

FileSize
4.36 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,417 pass(es). View

Patch applied cleanly for me. Attaching a minor update that adds a "title" class which gets bolded.

YesCT’s picture

Issue summary: View changes
FileSize
1.09 KB

@ohthehugemanatee Thanks for fixing that.

Often reviewers have reviewed previous versions of a patch, and adding an interdiff that shows just the changes between a patch will yield faster re-reviews.
Instructions for doing an interdiff https://drupal.org/documentation/git/interdiff

Here is one for the diff from comment 88 to 91.

------------

Updating the remaining tasks in the issue summary to include current 8.0.x head before and after most-recent-patch screenshots, leaving the novice tag for that.

YesCT’s picture

Issue summary: View changes
Issue tags: -Novice
FileSize
290.56 KB
268.99 KB

screenshots updated and put in issue summary.

next, manual testing, per @tim.plunkett in #73

YesCT’s picture

Issue summary: View changes

oops. really took out the out-of-date before screenshot this time.

Bojhan’s picture

Just wondering whats happening with the table header styling. Doesn't seem to match the styling we have for normal listings (e.g. admin/content). There seems to be a double top border.

YesCT’s picture

YesCT’s picture

Status: Needs review » Needs work

did manual testing.

the search filter works with the patch,
but
the type dropdown filter does NOT filter the results.

so marking it needs.

we cannot have a test for this because we do not have front end javascript tests.

yoroy’s picture

xjm’s picture

(Saving proposed issue credit for discussion and triage participants at LA, as well as other reviewers/testers/etc.)

dawehner’s picture

What can we do in order to move forward here?

Lendude’s picture

I think the remaining issue is the 'type' filter select not working because of its dependence on #states.

So options I see:

  1. Stop using #states and add javascript to replicate its effect for filtering by type
  2. Add #states support to the #type => 'table' on a per-row basis
  3. Stop using #type => 'table' and make a table-like layout using CSS

Since the text search is custom javascript too, looks to me like extending that to include type filtering would make the most sense in this case (plus the other options sound really bad :-).

Any other options to consider?

Bojhan’s picture

Issue tags: +Novice
Maouna’s picture

Assigned: Unassigned » Maouna
Zekvyrin’s picture

Re-rolling the patch for current drupal-dev (to share it)

Maouna’s picture

Having a look at this issue with veronicanerak and Zekvyrin, we found some more open tasks.

The checkbox for (de)selecting all is added via tableselect.js, but is not working as intended yet.
1) When the list is filtered, this checkbox should only act on the visible table rows.
2) The display of the selected items is not updated, which can lead to confusing output.

I will keep working on that.

Zekvyrin’s picture

Just an update from the previous patch.

What we've also found is that there are (up to) 3 issues to be resolved:

1) Firstly, Group filter is still not working. I'm still working in this one.

2) We've discovered 2 bugs from the "select all" checkbox in tableselect (the ones Maouna mentions in her post above)

I also want to add that one possible solution to make patch shippable is to remove the "select all functionality" by adding on the tableselect:
'#js_select' => FALSE

I have included that in the patch (for now).

I have also made some progress for the first issue and I will post an updated patch later. I'm trying to add custom javascript to filter by group (& I have removed states).

Zekvyrin’s picture

FileSize
6.79 KB

Well, another -incomplete- update.

I've managed to filter by group, but each filter doesn't actually takes into account the other filter, which is bad.

I need more time to "merge" these, but for now I'm posting an updated patch with my progress so far.

I've also included a css fix for odd/even table rows

Maouna’s picture

Here are two interdiffs, so it is easier to follow the progress.

@Zekvyrin: you can assign this issue to yourself.

Maouna’s picture

Status: Needs work » Needs review
FileSize
7.67 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,332 pass(es). View
3.72 KB

I merged the two filters into one. Lets see what the tests say.

Maouna’s picture

FileSize
8.61 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,333 pass(es). View
1.53 KB

Updated the patch so that problem 2) of #105 is fixed.

Maouna’s picture

Status: Needs review » Needs work
Zekvyrin’s picture

Status: Needs work » Needs review
FileSize
7.76 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,512 pass(es). View
494 bytes

Gj Maouna. Thanks for the updates.

I've checked & tested both patches and here are my notes:

First of all, patch #109 is shippable in my opinion and it can be committed by itself. That is exactly what I would do as well.
It doesn't include the "Select All" checkbox which still has some issues, and we can create a separate issue to fix those (several people in Friday sprints recommended it). As this issue is considered "Major" I would vote to commit it and create a new issue to include the "select all functionality". so +1 RTBC for this.

About Patch #110 and the "Select All" functionality:

1) One issue still stands : When you have filtered the table, if you click the "Select all" checkbox, it will select all the items (including the hidden ones).

2) I tested the patch and the approach, and it wasn't what I expected (removing everything from "selected" list below) but it can be considered a solution (and I'm not a UX/UI expert, so I'd leave it to someone else to check on this).
If we keep this solution I have one question/notice: What should happen if we check "select all" when the list is filtered (if the previous issue has been fixed)? I think it should show the "selected list".

3) Finally, a small coding standards notice:
handleCheckAll 's comments need a small fix (a space between "//" & the comment, and I'm not sure about the inline comment next to "if"):

    if ($target.is(':checkbox')) {//is added by tableselect.js
      //no matter whether the checkbox was checked or not checked
      //because it does not makes sense to list all options

Also, I'd like to make another comment about css:

By using tableselect, I've noticed that when the checkbox is selected, the whole line gets highlighted (by tableselect's css).
This characteristic wasn't happening before including tableselect and it isn't happening at odd rows because our css code was overwriting it (to create odd/even zebra rows):
.views-filterable-options .filterable-option.odd td

I've updated #109 patch by including the same row which overwrites even lines as well, although I'm not 100% sure that's the correct solution.

Also, by seeing all these I think that this task is not such a "Novice" Task... (so I think tag should be removed by that's minor:) ).

penyaskito’s picture

I've tested #112 and worked successfully, both search filter and type filtering. After playing a bit with it, the only thing I missed was the ability to remove a hidden selected item after being selected when I filter further, but that is an issue too with current HEAD.

Also, when clicking on the title, the checkbox is not switching its state, which is a feature we may not want to lose.

Zekvyrin’s picture

Assigned: Maouna » Unassigned

@penyaskito:

About the first one, do you mean that we should add a mark or something to "selected" list below?
Because the other way I can think of (removing checked automatically when filtered) is not something I would prefer and I think it's intentional not to remove them.
Personally I have searched & added many items at the same time by searching for each one, and I wouldn't like it.

About the second (clicking on title makes the checkbox checked), yes indeed it was present previously because title was checkbox's label.
If we need to implement it, should we add some js code to replicate it or is there any other way with tableselect? (I couldn't find one)

dawehner’s picture

1) One issue still stands : When you have filtered the table, if you click the "Select all" checkbox, it will select all the items (including the hidden ones).

Honestly, I think its 100% pointless to have a select all functionality.
For a normal site builder there is no usecase in having that, so ideally this could be dropped?

Bojhan’s picture

Yup, lets drop it.

penyaskito’s picture

@Zekvyrin That's it, a way to quickly remove them. However, that should not derail this issue.

IMHO, we just need to figure out what to do with clicking on title for checking/unchecking. If we can live without it, let's get this committed.
In any case, this is a lovely improvement. Thanks all who worked on this :))

+++ b/core/modules/views_ui/js/views-admin.js
@@ -486,30 +490,32 @@
+    handlefilter: function (event) {

Rename to handleFilter?

Lendude’s picture

Issue tags: +JavaScript

Per the D8 fapi documentation, both removing the 'check all' and adding a label should be supported by tableselect out of the box

Other settings:

  • To set accessibility tags for the radios or checkboxes, build one of the cells in the #options property using the 'title' => array( 'data' => array(#title => 'mytitle'))) construction, as in the $options variable in the usage example below. Drupal will create invisible label tags for the left column based on this value.
  • To disable the default "check all" button for the checkboxes, set #js_select property to FALSE. This is FALSE by default for radios.

So both things should be possible to add fairly quickly.

Added javascript tag because of the javascript changes that are made.

Lendude’s picture

FileSize
3.64 KB
7.62 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,483 pass(es). View

Nevermind on #118, #js_select is already FALSE in the current patch, so that option doesn't show, so that's good per #115.

Tried to get the 'title' => array( 'data' => array(#title => 'mytitle'))) thing to work.

- Switched to tableselect element (currently using table with tableselect => TRUE, not sure why)
- Switched the #options around a bit to get it to work with tableselect element
- Added the 'title' => array( 'data' => array(#title => 'mytitle'))) notation
- It only adds a usability label to the checkbox, not to the first column like I was hoping (but on reading the documentation more carefully, I does what it says)

This is not what I was hoping it would do, but would say still a usability increase over #112

Also fixed #117

Zekvyrin’s picture

@penyaskito: you are right about handleFilter and thanks @Lendude for fixing it.

1) I don't know why we were using table with tableselect TRUE. That's how the previous patch was working, and I didn't question the choice.

2) I also saw that in Form API when searching earlier but I read that it only adds a invisible label to the checkbox. Has some uses, but still doesn't do what penyaskito said about label. But is it something we 100% need to commit it?

3) About select/check all, well it may not have many use cases, but some might like it if it selects all filtered (for example selecting all node's fields) so it might be a possible addition for the future. For now, what I see is that it creates more trouble than what it is solving & it's definitely not blocking so that's why I think we can open another issue when this is committed.

dawehner’s picture

@Zekvyrin What about adding a new issue to discuss the additional of a select all checkbox? For now most people seem to not have a usecase for it,
so let's try to get it in as most people expect it to be like.

catch’s picture

Yes please open a follow-up for select-all - we should not add that here.

Zekvyrin’s picture

Follow-up issue opened: #2579171: Enable "Select All" Checkbox on Views UI's add fields new layout
So now we can focus on fixing it without the "select all" functionality.

#119 is working and could be RTBC as it is now.

The only question is if we want to "keep" the ability to check the checkbox by clicking the title.

Previously title was checkbox' label, but now it is in a different table cell. So if we want to keep it we probably need to remake the functionality using custom js (unless there is another way with tableselect but I haven't found any).

dawehner’s picture

The only question is if we want to "keep" the ability to check the checkbox by clicking the title.

Really good question!
I can't speak for all people, but at least personally I got really used to that nice behaviour.

nod_’s picture

Assigned: Unassigned » nod_
Status: Needs review » Needs work

Clean-up incoming, stand by.

nod_’s picture

Status: Needs work » Needs review
FileSize
8.09 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,981 pass(es). View
5.86 KB

Removed the striping of the table (per style guide), lewis confirmed we should remove it.
Refactored the code to make it nicer to read.
Fixed the click on title checks the box (not very pretty but it works, anyone with better solution is welcome).
Added the description class since searching on description terms didn't work before.

nod_’s picture

Assigned: nod_ » Unassigned
FileSize
2.32 KB
8.63 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,000 pass(es). View

For all those search fields we have a dedicated event that triggers a sane amount of time (not every keypress): formUpdated. It was made for this use case so used that one instead of keyup/change.

Think I'm done here, carry on.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing

Manually tested the patch. I think in a follow up we could make the entire row clickable but for now the title is certainly enough.
It indeed looks much better now.

Bojhan’s picture

Whoo :)

Zekvyrin’s picture

Manually tested it and it's working fine to me as well.

I also noticed a difference/small delay in filter between before 128 & after: indeed the event triggers less frequently than before.
Now the form needs ~half a second to be updated.

Well, to be honest as a user I'd prefer it like it was before as the time to filter the form is noticeable, but I don't think this should "block" the commit. And as a developer I understand the reasoning behind formUpdated event, so I'm not against it.

nod_’s picture

It's 300ms actually :)

xjm’s picture

Assigned: Unassigned » xjm

Reviewing this.

xjm’s picture

Assigned: xjm » Unassigned
Status: Reviewed & tested by the community » Fixed

Thank you very, very much for carrying this issue for three years. I just tested it manually. It's so much nicer. It works beautifully.

Committed and pushed to 8.0.x.

  • xjm committed d5c827e on 8.0.x
    Issue #1832862 by jerdavis, nlisgo, Zekvyrin, Maouna, nod_, royal121,...

Status: Fixed » Closed (fixed)

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

Gábor Hojtsy’s picture

Version: 8.0.x-dev » 9.x-dev
Issue tags: -sprint

Thanks, removing from UX sprint now.

Gábor Hojtsy’s picture

Version: 9.x-dev » 8.0.0

Fix version, now that 8.0.x does not exist.