Add a name and email search field to the admin/people view

I personally find this to be a really overwhelming set of handlers, though the username and email fields are definitely something I want 80% of the time I go to this page. Let's drop the items per page, at least.

Why two seperate text fields? Can it be one where I enter either user name or email? Does 'Active' map to status? Then it should say Status.

I'm not familiar with these filters, I wonder how useful the Permissions one is.

I agree that items per page does not belong there as a filter. If we want that behavior, then we better provide this as a generic, reusable one available in all listings: #2020449: Provide a generic/reusable widget for configuring the number of displayed items per page in admin views listings.

I also agree with merging the name and email in a single text field. BTW, why the "User Name Contains" label while email is simply "E-mail" and why use title case?

If "Active" was chosen because the drop-down is a yes/no one, then #1998192: Allow the boolean labels of exposed filters to be configurable ;)

Views can merge multiple exposed filters into one and search on all of them, but the question is whether this confuses the people. I haven't seen this pattern anywhere in views yet.

Google uses a single text field to search the entire interwebs :-) Having more than 1 field for entering a search term is overall the less familiar pattern and more likely to confuse than a single point of entry. At the very least it slows people down a bit because they have to choose between the two.

Yeah, I think "Name or e-mail contains" would be fine as one field.

I'm pretty sure we have the ability to combine two exposed text filters into one.
But I don't think it allows more than 2, which might be frustrating.
Just a heads up. (@dawehner correct me if I'm wrong).

What do you think. This is views, so it's flexible.

Yeah, we def have the Combine filter. IIRC you can select more than 2 aswell, as it just iterates over the selected fields.

This is #11:

I think this field is a great improvement.

We probably need to do some work with the arrangement of the fields, since there's sort of a chaos of buttons. The + Add user button sort of gets muddled in, especially in overlay, and it's not clear what the difference is between the filter set and the action dropdown. I think all that's out of scope here, though, so retitling for our specific goal in this issue.

@tim.plunkett linked me #1475204: [META] Provide a generic search/filter UI interface pattern for listing pages.

@#14: I've added a header to the view to help separate the Add User button from the filter options. I used the text from the D7 admin/people page but am open to other suggestions.

It would be ideal if we could put the filters into a frameset (like D7 does), but Views doesn't currently have any option for wrapping the exposed form in an element. For that matter, there isn't the option to control the label (above/inline/hidden) which some might expect given the Field UI.

See also #1812048: Build the exposed form using form API functions which is hoping to rewrite the exposed form rendering code to use FAPI. I need to get some screenshots for that issue before it can move forward... Hopefully later today. That would make it easy to add label placement options to Views for a start.

Isn't that quite a lot of space until you come to the actual filtering?

+++ b/core/modules/user/config/views.view.user_admin_people.ymlundefined
@@ -898,6 +898,19 @@ display:
+          content: "<h4>Show only users where</h4>\n"
+          format: basic_html

I think we should better use the custom text which does not require you to specify an input format.

Agreed, we should use an unrestricted text field rather than one tied to a text format which may change to disallow the tag we're using. The attached patch fixes that.

Isn't that quite a lot of space until you come to the actual filtering?

Do you mean that we should use something other than an h4? I would've picked an h2 or h3 (which have even more spacing!) as that is more standard, but that was disallowed by the basic_html format. I'm not sure I understand your meaning...

(edit: interdiff is between this and the previous patch, not this and #11)

ggrrr...

+++ b/core/modules/user/config/views.view.user_admin_people.ymlundefined
@@ -804,6 +894,22 @@ display:
+          content: '<h4>Show only users where</h4>'

To me it seems like this whole header should just be removed :/ I think it clutters up the UI and it seems pretty obvious that the exposed filters are for...filtering. If anything there is a case to add additional markup options for exposed forms, to wrap them but this title, I'm not getting.

+++ b/core/modules/user/config/views.view.user_admin_people.ymlundefined
@@ -831,4 +937,5 @@ label: People
+uuid: ''

Just remove this key entirely.

Also, from experience with admin_views I think it also makes sense to use the remember option to these? Either way, it's worth a discussion I think :)

Let's discuss the title in a follow up and write some tests for the filtering.

+++ b/core/modules/user/config/views.view.user_admin_people.ymlundefined
@@ -608,7 +608,97 @@ display:
+            identifier: combine

+++ b/core/modules/user/lib/Drupal/user/Tests/UserAdminTest.phpundefined
@@ -48,6 +48,17 @@ function testUserAdmin() {
+    $this->drupalGet('admin/people', array('query' => array('combine' => $user_a->name)));
...
+    $this->drupalGet('admin/people', array('query' => array('combine' => $user_a->mail)));

This is extremely nitpicking, but i think /admin/people?combine=yourname is a bit weird, do we want to change the querystring identifier?

Let's use 'user' instead, as suggested by tim.

Great, thanks @dawehner! It's great to have knocked this out so quickly.

Thanks @mikeker -- I think we can address the issue of fixing the header in a followup; it's out of scope for this fix since the problem already exists before this patch.

Filed #2023683: Improve the layout and usability of the admin/people exposed filters and actions.

Uhoh, it's broken.

STR:

1. Install 8.x standard with the patch applied. Use "admin" as the user/1 username.
2. Go to admin/people.
3. Enter "admin" in the "Name or email contains" field and click filter.
4. There is no result.

I tested a few ways and it seems to be matching only the email, not the name. So we need to fix that, and we need more test coverage for it. Not sure if it's a problem with the implementation or with the field mergy thingy in Views itself.

Gave #24 a quick spin at simplytest.me and found it not quite working as expected:

- Navigated to /admin/people
- Entered "test" in the name/e-mail field
- Hit the "Filter" button

admin (user 1) got returned as a valid result while the list should be empty.

...just saw xjm's #28. So perhaps the filter does the opposite and is a "does not contain" field actually.

...speaking of which, does anybody else think that we should provide a means for people to invert the filter(s) (for example allowing admins to filter people that are not of a specific role or that do not have a certain permission)? This could either be done by pairing the drop-downs with an extra "is/is not" drop-down or by using a "invert filter" button.

Curious if the tests included in a given patch are run as part of the tests of that patch? Because the test dawehner added should've caught this...

Anyhow, looks like the user:name filter got dropped from the combined filter at some point. (*#&^@&$ multi-select HTML elements).

...just saw xjm's #28. So perhaps the filter does the opposite and is a "does not contain" field actually.

If so, it does only for the name. Email addresses match.

#31 is out of scope here; maybe file a separate issue if you'd like to explore it? I'm not sure we want to add that additional complexity, but we can discuss it.

We just changed the exposed filter identifier from 'combine' to 'user', are you sure it was a clean install and whatnot?
I just retested this, it matches both username and email address.

Tentatively re-RTBCing.

Thanks @mikeker! NR for the bot, but we still need more extensive test coverage.

xjm @26: No problem. Let's get the bulk of this in and then fiddle around the edges later... Besides, I've been thinking of a long list of exposed form improvements I'd like to make! :)

Oops... xjm is too quick.

@tim.plunkett, it was on simplytest.me, so yes, it was a clean install.

Major cross post.
The interdiff in #32 makes sense, but I guess i'm just matching on mail, not on name (I have admin@example.com as my mail)

The interdiff in #32 makes sense, but I guess i'm just matching on mail, not on name (I have admin@example.com as my mail)

I guess I should have provided better STR. ;)

we still need more extensive test coverage

I just took a look at the existing tests and we hit everything except the Active filter. I'll look into adding that to the coverage.

@mikeker as you pointed out in #32, name was left out of the combine filter. So how did the tests for name pass?

@tim.plunkett, that was the root of my question: are the tests that are added in a given patch run against that patch by the testbot? Or does the testbot only run the currently committed tests?

I don't have enough experience with core dev to answer that... I appreciate any insights.

Oh, the patch is applied to core and then the tests are run.

@tim.plunkett: thanks for the clarification. I assumed that was the case, but wasn't sure why the earlier patch passed the testbot... Digging deeper, it seems there's a problem with WebTestBase->drupalCreateUser() -- it uses the username as the basis for the email address:

$edit['name'] = !empty($name) ? $name : $this->randomName();
$edit['mail'] = $edit['name'] . '@example.com';

Pretty much the same thing as happened in #39!

Anyhow, attached is an updated patch that includes tests for the active filter (the only filter left that we don't already test).

Crap. Wrong patch...

(Interdiff is still the same).

Ah jeez... It's time for bed....

Okay! That makes sense now.

oh.

Perfect

Removed tag

...
- Entered "test" in the name/e-mail field
- Hit the "Filter" button

admin (user 1) got returned as a valid result while the list should be empty.

Stupid me! admin default email in simplytest.me installations is in the form of [random_string]@simplytest.me. Of course it would match "test" as field input.

...btw #1998192: Allow the boolean labels of exposed filters to be configurable is now in so we could change the Active Yes/No drop-down to Status Active/Blocked. Before #14 this issue's title was "Improve admin/people view". So, do we do that here or do you want me to file a separate issue for that?

#31 is out of scope here; maybe file a separate issue if you'd like to explore it? I'm not sure we want to add that additional complexity, but we can discuss it.

#2024371: Provide means to invert filters in admin listings (admin/people admin/content etc)

Confirmed that it works now. Yay! This is why we put views in core, people. :D

Committed to 8.x. Thanks!

https://twitter.com/xjmdrupal/status/347799142183075840 :)

THANK YOU!

A minor language issue: "Name or e-mail contains". An e-mail is a message. We are talking about email addresses here. Also, the style guide says to write "email" without the hyphen. My suggestion: "Name or email address contains"

Oops.

There are 257 usages of e-mail in core (compared to 741 of email), open a follow-up. Thanks!

Our content style guide specifies "e-mail," so actually don't file a followup. ;) https://drupal.org/style-guide/content

I've also filed #2024893: Change status filter on admin/people to 'active'/'blocked' for #52.