Incremental filter for permissions page
dmitrig01 - March 2, 2008 - 22:50
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | user system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | dmitrig01 |
| Status: | needs work |
| Issue tags: | Needs usability review, ui-pattern, ux |
Description
This patch provides search/filter for the permissions on the access control page. With javascript disabled, the page looks the same as before.
Original idea by chx, done by me while sitting next to him
| Attachment | Size |
|---|---|
| incremental_filter_access_control.patch | 0 bytes |
| Testbed results | ||
|---|---|---|
| incremental_filter_access_control.patch | failed | Failed: 9747 passes, 3 fails, 0 exceptions Detailed results |
#1
Real patch this time
#2
There is a bug and this is a bug in jQuery - .clone doesn't clone input values
#3
This feels like it's missing something like a notification of what just happened. I accidentally typed "zbs" and the entire screen went away.
Maybe something like:
<strong>0</strong> permissions found using the term '<strong>zbs</strong>'. <a href="">Refine your search</a>?Clicking "refine your search" would bring the entire page back.
Also, there's no indication that typing in that box will filter the results. The label is "Permissions"... how about "Filter by keyword:"
#4
New patch; There's a nice "empty" screen and it now highlights the term searched for in the permissions.
Also, it only searches titles. I can add 2 lines of code for it to search descriptions too, so opinions?
#5
I haven't actually tested this patch, but I think it should take into a few issues right now:
- It seemed really slow when you showed it off today.
- The highlight should definitely be something more definitive, like red or bright green. Not pale yellow (Jimmy, who is sitting besides me, suggests seeing what the CSS file uses so it adjusts properly).
- Definitely add searching of the descriptions.
Overall, a huge +2, if it's implemented properly.
#6
Yellow is the conventional highlight color. Green I could see, but red means "error", which this isn't.
I'm also +1 on searching descriptions, too.
I haven't noticed a performance problem myself, but I didn't benchmark it.
#7
Shouldn't the interface strings present in Drupal.userPermissionSearch (and possibly others) be run through Drupal.t()? Possibly these need to be themable as well but I admit to not looking closely at that.
#8
Bug: after scrolling and when it's inside the sticky tableheader it breaks when the list has been filtered down and the sticky table header is removed. I can see the rational of having it next to "Permissions" in the header, but instead of dealing with that edge case it migh be a better option to put it above the header. Or maybe hide it when the sticky tableheader is initiated.
There should be a label, either inline (e.g grey text which disappears when clicking inside the textfield) if inside the tableheader, or a standard label if displayed above.
To increase performance, don't filter on every key down, just after a specified delay, e.g 300ms. Core autocomplete does this.
This kind of filtering could be useful on the modules page as well. Nice work!
#9
reminiscent of http://acko.net/blog/jquery-menu-scout. that became a patch and then a contrib module named 'menu scout'. perhaps borrow some ideas from there if applicable.
#10
Like it, a lot. Definitely nice to have this work on descriptions - people will search for something they remember which may well be in there. Haven't tested the patch yet, but I saw the demo and it looked like fine work.
#11
Here's my submission based on #4.
- Common value between the actual and sticky table header. Shouldn't this be handled by tableheader.js (Possibly this is a bug like on node and user administration pages having a checkbox in header).
- Filter by description as well as highlight.
- Added mouse up event.
- Case insensitive search, using "i" flag for regular exp. match.
- Hide Save permissions button, when no permissions are displayed.
One thing I encounter with this patch is about highlighting. Type "administration" and administ part of administer will also remain highlighted. This one arises from the two if()s for the title and description. Have to put time elsewhere, so will put the rest as needs review state.
#12
Here's a screenshot for an overview of the feature.
#13
The "No permissions were found" text needs to be translatable.
#14
Barry has a similar patch which is going to be a Drupal 5/6 contrib module AFAIR, which he used in his 3 minute web 2.0 demo, to quickly filter the module list down. So it would be great to have this as a generic filter script. Video: http://blip.tv/file/856641/ (look at around 3:00 for that feature)
#15
FYI, the module I wrote that Gabor is talking about is called Modulator. Currently it is just a filter for the modules admin page, but I also want to change the modules page from a list of checkboxes to a list of icons for installing, enabling, disabling, and uninstalling modules without having to submit the form. Haven't gotten to that yet, do not know when I will.
I intend the module's functionality to be a patch for D7 core but needed for my Launch Pad demo on D5 first. :-)
#16
bjaspan, aren't both wishlist items independent? We might be able to commit them as separate patches.
#17
Dries: I think, these might be possible to generalize to a widget which would be useful elsewhere in Drupal as well. A live search widget for tabulated data. It might not make sense to generalize it, but these two seemed very similar to me from the looks (did not check the code, esp that Barry's code is not yet available).
#18
@Gabor – Konstantin and I talked about this about Drupalcon.
The widget would be hard. Here's the challenge, I hope you understand.
Supposed you are searching this table cell:
<td>Testing<strong>(some code)</strong></td>So you search for
Testing. It highlights so it comes out like this:<td><span class="highlight">Testing</span><strong> (some code)</strong></td>But. What if you search for
ing<strong>?<td>Test<span class="highlight">ing<strong></span>(some code)</strong></td>That's invalid HTML. But. If you search the .text(), then the HTML gets stripped.
Now, you say, "Let's try something here: search the .text() and inject it back into the HTML somehow".
So it will search
Testing(some code)What if you search for
ing(some?That's what we need to figure out, I have no idea.
#19
dmitrig01 - who's going to search for
ing<strong>though?#20
@#17: The code for my module search is available and has been for a while: it's the Modulator module in contrib. No releases yet and I have not created a project page for it. Also, I don't claim the good is any good; it was a quick hack for the Launch Pad demo. But it *is* available. :-)
#21
@catch I don't know, but it's possible :D
#22
@dmitrig01: and is the only issue that you get invalid html while filtering? I really don't see this as a reason to hold this up.
#23
@catch more practical problem: You search for ron (part of cron), and get
<st<span class="highlight">ron</span>g>#24
Not easy. But then again, not that hard either. Just a bit of string juggling and a mighty preg_split to prepare for the search.
#25
So here's an explaination for what the above does.
You want to search for "llo test" within the HTML
hello <em>test</em>, and you want to highlight the resultng string.There are three ways:
(a) Search within the HTML string.
This is bad because you get this:
he<span class="highlight">llo <em>test</span></em>, which is invalid html.(b) Search within the stripped string
This is bad because you get this:
he<span class="highlight">llo test</span>, which makes the formatting disappear for some reason.(c) Search the way we do it
It's really really complicated but it works, and doesn't have the other deficiencies.
#26
So the complicated way with much less optimization (which clouded a lot of things) and much more self-describing variable names.
Edit: that optimization was needless micro-optimization of trivial arithmetic so it's not detriment to performance to remove them.
#27
Re-rolled with chx's code included (but ported to JS) and documented.
#28
and a demo
http://dmitrizone.com/229193.mov
#29
oops contained cruft
#30
I would like someone from the UX team to chime in on this patch. If this UI construct is found to be desirable, I would also like to see this abstracted out and added to some of our other 'mammoth' pages (notably, ?q=admin and ?q=admin/build/modules).
#31
I've added this to http://groups.drupal.org/patch-spotlight
Tried the demo and it's very nice, I also agree this'd be good to abstract out as a general js table filtering pattern for other pages.
#32
I get malformed patch at line #508 when applying this, also looks like it needs to be made to work with uppercase letters - 'A' leads to 'no permissions found'.
#33
Searching for permissions? That doesn't sound like the right answer to the underlying usability problem.
I mean, when do users have to search? I guess they want or have to, if the total result set of all possible options is too large, or if the answer is yet unknown. Drupal permissions, however, are pretty limited. Of course, there are many permissions if one installs a bunch of contrib modules. But then again, users would probably want to view them grouped by meaningful topics, rather than searching for arbitrary strings. For example, we could re-use the existing grouping by package name, just like we do on admin/build/modules. And we could display permissions of recently enabled modules separated/first, in front of all other permissions.
#34
I had a quick talk with yoroy, about this there are one main concerns :
You need to know what you are looking for
This is very efficient for those who know exactly what they are looking for and even if you dont, just filling out a couple words would probally already show up your result. However if you happen to have a high ammount of modules, this might not scale well - because you cant overview if you dont know exactly how to word it, would autocomplete on sites with a lot of modules be possible / help ?
Apart from that the contrast is just right (aesthetics less on descriptions). We might need to rewrite some of the text, as some of it is non-descriptive of what it is actually about - but that can be adressed in a other issue. Also typo's and shortcuts, if its aimed at efficientcy its worthwile to look into.
Other then this, I and yoroy like it. Even though though some concerns, it can be added to core.
On the notion of using it elsewhere, sounds good, but you have to concider that on this page the task is to find a specific permission, while on other pages there might be a more important goal, that should be adressed by the search.
sun: We are avoiding an underlying usability problem, but thats more concernd with the broken workflow of installing a module then with this screen. It's obvious that for beginning users, this functionality might not apply (would have other search pattrens) - but this seems aimed at intermediates/experts.
#35
I'm not sure if I think this is a good idea or not.
As Bojhan says this is clearly aimed at the intermediate/expert crowd, so if there is a consensus that this is going to be useful to the kind of people who will read this issue then I think it is fine. We will, of course, have to make sure it doesn't confuse beginners, but we can probably accomplish that easily enough.
However, while it looks like it might be useful, I'm not totally convinced it will be in practice. You already have a search function built into your browser; I know it doesn't _filter_ the search, but do you really need it to actually filter? Does search highlighting not do the job?
In my opinion this addition would need to be justified a lot more.
#36
Well the search in the browser (the same case, without filtering results) requires the user still, to scan the whole page for highlights (in firefox's case, click next) - which makes it quite inefficient. We can just test it with beginners and possibly also intermediate in a usability test - to find out whether it really confuses.
Alpritt not sure if your using firefox, but that's where search is quite broken for these kinds of lists.
Apart from that its important to differentiate from known-item seeking (information that you know how its called) and want-to-be-known-item-seeking when you don't know exactly how its called but plan to find it by looking at the content. In which case a filter like this, is faster and easier then firefox search.
#37
I use ctrl-f on the permissions and modules pages most of the time. Especially on the modules page (where I hope we could apply this next), it's completely broken due to having module names showing up in dependencies - so in some cases could take ten or fifteen clicks to find. As such I think this is a worthwhile improvement.
#38
If it will be able to deal with that scenario, I believe this is a good enough reason to have this feature.
Responding to webchick's comment, I don't think it would be appropriate for /admin. The modules page and the permissions page are by their nature long lists of things, so it makes sense to search through them. While /admin is a long list of things too, by its nature it shouldn't be. There are better ways to fix that page.
I'd love to see this work on such things as long lists of content; although I'm not sure how practical that is considering the paging. It's out of scope for this issue, but I wanted to throw it out there anyway.
Are we intending to abstract it in this patch?
#39
Ooh, this would be very useful for the modules and permissions page. Subscribing.
#40
Rerolled, this works.
#41
#42
Here's with the header Filter: as suggested by the usability team.
#43
Screenshot of the difference
#44
@dmitrig01 there are other things in this patch besides the permissions page changes - vertical tabs and user settings page picture support stuff. I'm assuming those aren't supposed to be in there...?
#45
Wow - the logic of the Drupal.userPermissionSearch function is awesome :-) Makes my head spin
I would just make a couple of minor changes, mainly in the comments, which I will do if you reroll the patch without the other changes I mentioned above. Also, seeing as there's so much in that one function, is there a case for abstracting it out into separate functions? I guess that's just a question of style, maybe not so important.
#46
@katbailey: You successfully made yourself ineligible to RTBC this issue! :-D
PNW until someone understands the logic who is not a jQuery ninja.
#47
Here's the proper patch.
#48
I was reading through, trying to make sense of the javascript, when I noticed that both of these lines need a period at the end of them:
// If it was found, begin highlighting
// The offset at which to start the highlighting
Also, tab on the following line:
+ td.html(output);Now back to the javascript itself. The logic is a little tricky at times, but I think that with the abundant comments, I can follow at least the basics of what it's doing. However, I think it might be easier to understand if the giant scary Drupal.userPermissionSearch function were to be split up into several smaller functions. That way, I could look at a nice clear central function and then look at any of the specific functions it calls without having to be concerned about the rest of them.
Also, maybe some kind of html nesting would make the process easier? That seems to be the reason behind a lot of the function's complexity.
#49
How about something like this, i.e. separating out the chunk highlighting magic from the main function that scans through the rows...
Drupal.userPermissionSearch = function(term) {
if (term) {
var previousModule = false;
var zebra = true;
var isEmpty = true;
$('#permissions > tbody > tr').each(function() {
td = $(this).find('td:first');
if (td.is('.permission')) {
var index = td.attr('id').replace('permission-', '');
// If we have found the text in the stripped string, get the first character position
// of the found string.
var found_begin = Drupal.settings.userPerm[index].stripped.indexOf(term);
// If it was found, begin highlighting
if (found_begin !== -1) {
var foundTerm = new Drupal.termInstance(term, Drupal.settings.userPerm[index], found_begin);
$(this).show().removeClass('even').removeClass('odd').addClass(zebra == false? 'odd' : 'even');
isEmpty = false;
previousModule = false;
zebra = !zebra;
td.html(foundTerm.output);
}
else {
// Text not found. Hide the row.
$(this).hide();
}
}
else {
if (previousModule != false) {
previousModule.hide();
}
previousModule = $(this);
}
});
if (previousModule != false) {
previousModule.hide();
}
if (isEmpty) {
$('#permissions > tbody > tr').hide().parent().append('<tr class="user-permissions-empty"><td>No permissions were found.</td></tr>');
$('#user-admin-perm > div > input#edit-submit').hide();
}
else {
$('#permissions > tbody > tr .user-permissions-empty').remove();
$('#user-admin-perm > div > input#edit-submit').show();
}
}
else {
$('#permissions > tbody > tr.user-permissions-empty').remove();
$('#user-admin-perm > div > input#edit-submit').show();
var zebra = false;
$('#permissions > tbody > tr').each(function() {
if (zebra) {
$(this).addClass('even').removeClass('odd');
}
else {
$(this).addClass('odd').removeClass('even');
}
zebra = !zebra;
}).show().find('.highlight').removeClass('highlight').end()
}
}
Drupal.termInstance = function(term, instance, found_begin) {
// Length of the search string.
var search_length = term.length,
// The last character position of the search string in the stripped string.
found_end = found_begin + search_length,
// The final, assembled string.
output = '',
// Whether the device is currently highlighting.
highlight = false,
// Whether the device has encountered the first chunk that contains the search string.
first_chunk = false,
// Whether the loop is about to encounter a tag.
tag = false,
// The offset, so far, between the string with HTML and string without HTML. Each
// time another chunk gets processed, if the chunk is a tag, this value is incremented
// by the length of the chunk.
tag_length = 0;
// Iterate through the chunks.
for (var i = 0; i < instance.pieces.length; i++) {
// Get the current chunk.
var chunk = instance.pieces[i][0],
// Chunk length.
length_of_chunk = chunk.length;
// If we're in a tag, add the tag to the tag_length variable, and do nothing else.
if (tag) {
tag_length += length_of_chunk;
}
// This is where the magic begins.
else {
// Get the starting offset of the chunk.
var start_of_chunk = instance.pieces[i][1],
// Get the end offset of the chunk.
end_of_chunk = start_of_chunk + length_of_chunk,
// Starting offset, but in the stripped string.
start_of_chunk_in_stripped = start_of_chunk - tag_length,
// Ending offset, but in the stripped string.
end_of_chunk_in_stripped = end_of_chunk - tag_length;
// If the first character of the found string is within the chunk's offsets in the
// stripped string, then we have found the first chunk.
if (start_of_chunk_in_stripped <= found_begin && found_begin < end_of_chunk_in_stripped) {
highlight = true;
first_chunk = true;
}
if (highlight) {
// If it's the last chunk, that means that the end of the search string is before
// the end of the chunk.
var last_chunk = found_end <= end_of_chunk_in_stripped;
if (last_chunk) {
// If that's true, then stop highlighting after this.
highlight = false;
}
// The prefix before the chunk if it's the first chunk.
var prefix = '',
// The postfix, after the chunk, if it's the last chunk.
postfix = '',
// The offset at which to start the highlighting.
start = 0,
// The length to highlight.
length = length_of_chunk,
// Whether we need to run a substring or not.
substr = false;
// If we're in the first chunk...
if (first_chunk) {
// We need to run substring to generate a prefix.
// Where to end the prefix, and start the highlight.
start = found_begin - start_of_chunk_in_stripped;
// Generate the prefix.
prefix = chunk.substr(0, start);
// We might as well set substring length to the full length of the search string -
// in the worst case, it will go to a longer length than needed, and there's no breakage
// that will happen.
length = search_length;
// Yes, we need a substring.
substr = true;
}
// If we're in the last chunk, we need to find the postfix.
if (last_chunk) {
// If we're not in the first chunk, the length is not the length of the
// search string, so calculate it.
if (!first_chunk) {
length = found_end - start_of_chunk_in_stripped;
}
// Get the postfix.
postfix = chunk.substr(start + length, length_of_chunk);
// And we need to substring.
substr = true;
}
// The text to highlight.
var chunk_to_highlight;
// If we need a substring, get it.
if (substr) {
chunk_to_highlight = chunk.substr(start, length);
}
// Otherwise, just get the chunk.
else {
chunk_to_highlight = chunk;
}
// Slap it all together.
chunk = prefix + '<span class="highlight">' + chunk_to_highlight + '</span>' + postfix;
// We're not in the first chunk.
first_chunk = false;
}
}
// Add the chunk to the general output.
output += chunk;
// If we weren't in the tag, we are now, and vise-versa.
tag = !tag;
}
this.output = output;
}
Either way, this is a terrific piece of functionality and it would be awesome to get it in :-)
#50
Here's how i'd do it
#51
OK, here's a new patch. @dmitrig01 I reverted that changed line
previousModule = $(this).show();back topreviousModule = $(this);because it was causing a js error (previousModule.hide is not a function) and it seems to work perfectly well like this.#52
This won't work. Search for "block", select the contents of the textfield, and type "content" (don't ever delete anything). previousmodule.show worked for me.
#53
Yep, you are right, I've just changed it and I'm no longer getting that error, don't know what that was about
#54
I think this is fine with working only if JavaScript is there. Drupal works without JavaScript but with reduced functionality already. You can not resize a textarea without JS for example. Autocomplete does not work. This piece is similar.
#55
More docs
#56
real patch
#57
HEre's a test module.
#58
better test module.
From basic testing with this module, the number of roles matters very little, what matters is the number of permissions.
#59
I'm optimizing it
#60
Here we go. This is quite fast. It works (not too fast, but it works) with > 500 items. PHP wouldn't let me test with 1000 table rows :-(
#61
When I type in "Administer", "Posts", "Select" I get no permissions found, where as it correctly filters words like "block", "actions","cancel" and numerics like 2,1 etc.
There are permissions defined with words Administer, Posts etc.
#62
Update, I typed in capital A in Administer, It takes in small "a".
#63
Sorry, thanks for testing. Fixed (about 10 character difference)
#64
with working zebra striping (a bit *too* much optimization :D)
#65
Minor typo:
+ * Highlight the instance of a term in a user permisison.s/permisison/permission
#66
Fixed. incremental_filter_access_control-229193-66.patch
#67
The last patch in #66 doesn't seem to work for me (Javascript errors). I'd like to test it out because I'm wondering how it is different or better from my browser's built-in search. My browser provides local search with high-lighting already. Curious!
#68
Fixed. sorry about that.
#69
fixed for real
#70
for real real
#71
Just noting we have a similar ui-pattern in /admin/build/path, but with a submit button and a page reload.
Go ahead making this work for permissions first, would like to see the filter on /admin/build/path work in a similar way in a followup.
Two different versions of (nearly?) identical functionality is not helping us smoothen the user experience.
#72
The fundamental difference to the path admin page is the amount of path aliases. Not even the biggest site will have thousands of permissions but any big site definitely will have hundreds of thousands of path aliases. So making the two filters different actually make sense.
#73
Actually, isn't that more of an argument to NOT do crazy in-line XHTML highlighting on some pages and not others, since there will be some pages that we can never do it for but users won't understand why?
#74
I'm coming in a little bit late to this one, but the jquery bug seems like a showstopper to me. Why not add a name or a class to the search box and add something like this to the keyup callback --
$('input.user-perm-filter', context).not(this).val($(this).val());Also, maybe I'm missing something, but I don't see where the delay to the keyup cb is happening in the latest patch in #70.
// Process the date and time, to make sure that there is a 100ms delay// between iterations of the filter, or otherwise fast typers would make
// the filter unbearably slow.
var time = (new Date()).getTime(), newTime = time;
#75
I think this looks nice, but it is not spectacularly better than my browser's search. It's quite a bit of code for a small improvement. How do other feels about this? I'm undecided.
#76
The last submitted patch failed testing.
#77
I've used the Firefox find-as-you-type feature on the permissions page since, well, as long as I can remember, and it's never failed to find what I'm looking for. I most often do a search for the name of the module, as in 'user module', just so I can avoid scrolling down the page for two whole minutes with my mouse wheel.
While I love the idea of this Filter By: feature, and I don't think it needs to be degradable in the least, I think that it would be faster for experienced users to simply use their browser's find-as-you-type. Having said that, if a user doesn't know about the features of the browser they use because they think that AOL is the internet, then having something like this in core is a great thing.
Biggest question? What's the real usability problem this widget is trying to solve?
#78
Senpai see #34, I think the notion of "browser" as replacement for functionality in Drupal core is kind of silly. Although we know this functionality in our browser, I think alot of users don't. Looking at the usability test we did in Baltimore only 1 out of 11 persons used the browser it's search.
#79
Where this would be really useful is on the modules page - where CTRL-F gives you completely useless results due to dependencies and requirements listings. If we could search on just the human and machine readable name for the module, and get to the specific row, it'd massively improve that page. Some modules end up listed on that page 30 times and CTRL-F becomes useless - only something like this which can inspect the specific HTML we output can get past that.
I also think it's completely fine for this to just degrade to the page as it is now - same as sticky table headers.
#80
@Bojhan in #78: I think that I'm saying exactly what you're seemingly contradicting me for, so I rather suspect that we're both talking about the same thing here. Cheers!
Now, I asked what the real problem is. The one that this new widgety thing is trying to solve. You pointed me to comment #34, where you said
So lets get down to the root of the matter. We need a filtration system in order to find what we're looking for on the 12 Mile Long Permissions page, right? Whether it be a Filter By: widget for beginners or a browser find-as-you-type like Dries suggested, we're all really just looking to narrow down our options before making the requisite adjustments.
But what if we could have arrived on this page with the prescribed permissions from, say, the Buddylist module already pre-filtered? Would that not allow a user to follow some inline, lead-me-by-the-hand help text link and come straight here to make the adjustment?
What I'm getting at is that if this is going to become core worthy, i.e. more than a replacement for a browser's inline search, it should have the ability to parse an %arg from the URL that allows the admin/user/permissions page to pre-narrow the user's choices by filling in the Filter By box for them. Ya know, in case they came here by way of a help text link. And if they didn't, well, they're free to use the Filter By box to narrow things down like a professional web developer might; all DX like and stuff.
#81
Shouldn't this be a behavior which could be applied to all tables? E.g.: by adding a classname js_sortable to a th?
#82
#396478: searchable modules page with vertical tabs!