| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | feature request |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | postponed |
| Issue tags: | d8ux, Drupal 8 priority, Usability, User interface |
Issue Summary
Problem/Motivation
On the modules page, it gets harder and harder to find / enable / disable desired modules, the more modules are installed in a Drupal site.
Proposed resolution
If we were to add a search box that could search the page with JavaScript people would be able to find the module they were searching for.
Remaining tasks
1) Enhancement of themed tables to contain a concept of "rowgroups" perhaps #31535: Grouping of rows in theme_table()
2) Introduction of a JS library or Drupal specific scripts to quickly search and filter a table
3) Addition of the search element and filtering JavaScript to the modules page
See #229193: Incremental filter for permissions page for a related attempt
User interface changes
- New search box added to modules page
API changes
- None
Original report by Frando
This patch adds vertical tabs to the module admin page, and search too! Awesome, isn't it.
Search is done via AHAH, and a new vertical tab is inserted containing the search result. Searching is fast, because we don't rebuild all caches as on a normal modules page load. The search matches module names and descriptions. The enter key is also bound to the search, makes it even faster.
Oh, and it degrades perfectly, the search button is totally usable without Javascript.
The patch looks bigger than it is, as it moves a big of code around.
The first patch attached requires both #396466: Support custom success callbacks in ahah.js and #323112: Vertical Tabs, for easier testing or functionality review use vt_modules_search_frh_combined.patch one which includes both depending patches.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| vt_modules_search_frh.patch | 9.43 KB | Idle | Failed: 10490 passes, 0 fails, 1 exception | View details | Re-test |
| vt_modules_search_frh_combined.patch | 31.79 KB | Idle | Failed: 10490 passes, 0 fails, 14 exceptions | View details | Re-test |
Comments
#1
screenshots
edit: in the patch I changed some strings after I made the screenshots. The vertical tab is actually called "Search results" in the patch, and the search fieldset at the top is called "Search modules".
#2
It has been noted in #drupal-usability that it would be great to better visually integrate the search field into the table. I will try to work on that tomorrow.
#3
The last submitted patch failed testing.
#4
New patch attached!
Thanks to the improvements in the vertical tabs patch, styling now is much much better in garland. I also added descriptions to the categories, and everything works quite a bit smoother (less flickering) because I improved the Javascript so that upon a new search (hitting enter key or clicking search) no new tab is created, but the existing one is replaced with the new one and the behaviours are copied over. Also improved docs.
The patch still depdends on #396466: Support custom success callbacks in ahah.js and #323112: Vertical Tabs, and the combined patch includes both other patches and goes directly against HEAD.
#5
screenshots
#6
The last submitted patch failed testing.
#7
Oh my - lets *please* refresh this patch and push it in.
#8
Some initial feedback based on screenshots:
Search is excellent and badly needed. All power users I know use their browser's find-on-page functionality.
I'm torn about using Vertical Tabs here. The positives are the added information ("28 modules, 9 enabled") and the reduced scroll length of the page. The downsides are that you can no longer scroll through all modules at once and there is less width for the descriptions, especially when enabled. For an example, see the enabled "Database logging" in http://drupal.org/files/issues/modulesearch4.png.
IMO, the tabs worked very well on node add page because it compressed and hid a number of options that can often be skipped. The spacing also worked because there was little within each fieldset. This use case is different and perhaps a little less clear. I'm not for or against the change at this time. However, I would caution that we need to set guidelines for when to use Vertical Tabs so there is consistency throughout core.
#9
Just a quick note that CTRL-F on the modules page as it is currently is completely useless - if you have modules with dependencies, you might have to scroll through 20 results (if searching for 'views' or 'i18n' for example) before you get to the module you actually want to enable or disable. So massive +1 for the search - hoping we can skip dependencies with it and maybe include system names for modules as well - but haven't tested this so it might already be in there.
Is there a reason that search and vertical tabs have to be in the same issue?
#10
search and vertical tabs are in the same issue because they probably touch the same code and then we would have dependant patches which are really hard to maintain.
#11
I'm definitely interested in a way to find specific modules in the modules list put into core. For those of you who don't know, Module Filter does this sort of thing for Drupal 6. Module Filter adds a textfield to the module list form that when typed into simply hides modules that don't contain the inputed string in the module's name, and when a fieldset has no visible modules listed then the fieldset is hidden. The module also provides vertical tabs, via a setting, with the first tab simply being a list of all modules in alphabetical order. With the tabs option and without, the modules are only hidden and not removed so any changes that are made are held when submitted.
I was looking to start migrating the module to Drupal 7 but found discussions of vertical tabs in 7 core and the possibility of searching the list of modules so I have held off and will help out if needed in getting something like Module Filter into core.
#12
So here is a reroll that is now based on vertical-tabs.js as it is in core now.
This actually made the patch get quite a bit smaller.
Screenshots remain the same as the last ones posted, functionality as well.
The patch still depends on #396466: Support custom success callbacks in ahah.js, which got much simpler now thanks to a good suggestions by quicksketch.
So for testing this, apply this patch and then the one attached to this post.
#13
I'm a bit concerned about the overall page width when we add vertical tabs. Maybe we should instead create horizontal tabs (the API is already there) or reduce the width of the tabs on this page.
#14
I don't think this patch makes the modules page any easier to use. It needs to be made simpler; (Read: remove shit from the UI), This patch only adds stuff to the UI.
I would suggest that this UI needs some more thought, research, design and prototyping. Perhaps just the search filter without out the tabs? What if it had live, as-you-type-it results? Does it search the module description too? Or just the module name? There are many other ways to present the modules page UI, and clearly collapsible fieldsets is not a very good one, but I'm not sure this is much of an improvement.
#15
tagging
#16
I need to review this later, however I am on the same page as Bevan. Mostly due to it not really changing the table, but soley the grouping. While the table is broken, even more then the grouping.
Marking it needs work, for this weekends sprint.
#17
#229193: Incremental filter for permissions page
#18
while this improvement in the works for the modules management would you consider 'client-side dependency checking' feature
-currently provided by a contrib http://drupal.org/project/qamodules
plus
a selector > turn off all contrib. modules feature (when the 'core -required-' modules tab on)
-especially needed for core before updates and during a maintenance work etc.
-currently provided by a contrib http://drupal.org/project/contrib_toggle
that would give yet additional ux improvement in the core without the need for contrib modules that provide those small but useful functions for modules management
#19
I think the incremental filter is a good idea here but the vertical tabs are not really. Let's do that later. And while it'd be better if we would be able to do a lot better page for now we do not have a better idea than the filter.
#20
+1 for incremental filter. Would be great to get a patch going without the vertical tabs before code freeze, unless there's already one lurking in another issue that I'm unaware of.
#21
http://drupal.org/node/538904 has more general modules page discussion and outlines goals we want to achieve when redesigning it.
#22
Changing the scope of this issue, to a search able module page. This seems like functionality that is required, when you have a lot of modules installed and if you wish to find your module (since in browser search doesn't work due to dependencies)
I'd like to refer to #229193: Incremental filter for permissions page , where a similar issue was trying to be solved. Lets make this a somewhat generic pattern so it can be scaled to other places.
#23
This is the initial version of InstaSearch.
There are three types of elements:
instasearch-container.instasearch-itembecomes an item.instasearch-ignore.Doesn’t tackle the zebra striping problem.
#24
The search field should probably be formatted in a more visually appealing way.
#25
Awesome patches in the Drupal 7 issue queue!
#26
We don't abbreviate CSS class names, so please rename "instasearch" to "instant-search". Otherwise, this looks quite promising.
I'm a little bit worried about the co-ordination of UX issues, since this is mainly targeting the modules page, but the modules page is about to be revamped.
#27
sun: having a filter like this is part of that revamp
#28
sun: the script is not specific to the modules page but could be applied to any page. It uses class names (as outlined above) to do filtering.
When we have tabledrag, why can't we have instasearch?
#29
I'm not sure if this warrants its own CSS file for just one rule, unless you're going to need to add a good bit more to it. Otherwise, seems like system.css would be better suited.
#30
Given that this is a new feature, I'd prefer to put this on hold until some more "Slush exceptions" made it in. We only have 2 weeks left, so let's make sure to focus.
#31
This is UX which should be fine even past slush.
#32
.
#33
Either way, I just tested it - it seems to work oke - however we could probably do with a better empty message. I am assuring the coordination of issues, is done well. This issue can be taken in separation from the other, it shouldn't influence its design.
#34
sub
#35
Still doesn't detect pastes correctly.
#36
Subscribing
http://drupal.org/node/598738#comment-2132216
#37
The last submitted patch failed testing.
#38
HEAD was broken. Resetting status.
#39
How about this patch? It's been discussed here: #598738: Modules page: add Search box for filtering by module name. It adds a tablefilter behavior to the Drupal behaviors object and attaches the behavior to the module table. Don't forget to put tablefilter.js into the /misc directory.
#40
The last submitted patch failed testing.
#41
#23 works pretty much the same as skilips patch, you enter some letters, and it searches in both title and description (I guess in any text in the table), and filters down the table accordingly as you type.
Still a label for the search field and the nice inline text inside the input that disappears as you start to type (as here http://www.web-source.net/javascript_disappearing_form_text.htm) would be better. So best combine the features of both patches ...
@kkaefer: could you please reroll the patch against the latest dev? There are heavy offsets meanwhile... Still I was able to apply the main part.
Also there is a css property in the reset part of seven that sais "border:none" for inputs that makes the search input field strangely invisible.
Would be great to get one of those in...
If it is rerolled, it would get an rtbc from me.
@skilip: you can add a new file in the way konstantin did in the first lines of his patch:
--- /dev/null
+++ misc/instasearch.css
by first removing it somehow with the /dev/null.
Not sure if only git can do that kind of patching... my cygwin with an -up flag could not.
So no need for manual adding of files and you get more reviews...
#42
added d7ux tag
#43
diff -upN or cvs diff -upN. But read up on adding new files here http://drupal.org/patch/create and this: http://wimleers.com/blog/cvs-diff-new-files-fakeadd
#44
Can anyone still work on this?
#45
@Bojhan we just gotta get someone to reroll one of the two patches.
#46
Rerolled Konstantins Patch.
It appears to work fine, also when you paste somthing into the search box.
#47
Why not use your browser's search feature?
#48
@Dries: CTRL + F is just a workaround for a usability problem of the module's page.
#49
@Dries Me or you or any geeky person can do that. But not less tekky ones. Actually even I have spent quite some time searching instad of using Ctrl + F.
Plus: this even works when you collapse the package groups on modules page with Admin Menu. It does not open the fieldsets, but it shows only those fieldsets that contain search results.
#50
Additionally, CTRL-F forces you to tab through false positives that appear "somewhere" in the page - notably hidden in admin_menu's menu tree.
#51
yes, the filtering down is nice and clean - leaves only few results on the page.
#52
This is a very lovable patch.
I would certainly like this. I use find in page and this would be a big improvement. I would be annoyed however if it was all hidden in vertical tabs.
The patch is very good.
I recorded myself using it for the first time.
http://screencast.com/t/Zjk1NjdiYzA Watch me struggle! fun!
At 1'43" I discover that the choices don't reset to default 'full' view when you clear the text field. This seems like it should. Otherwise subsequent searches can give empty results when there is something there.
At 2'50" - 3'24" I discover it searches the modules, but not the collapsed container titles. Like, what if I am searching for CCK, not Content. There may be a grouping of modules I want to go to like 'Other'... Does that matter?
#53
I can confirm the filter does not read Fieldset labels, but only inside the tables. So, e.g. filtering for "core" gives an empty result.
The resetting (and thus viewing all fieldsets when emptying the box) works fine from me, if I delete the letters one by one.
But If I highlight all text in the box and then delete it, there is no reset. Only a page reload resets the page then.
#54
Eigentor called me on something. I put in the .js from an earlier patch... because... well i get confused sometimes. I thought it should be in there.
So: just to prove the problem... I made yet another video, this time shorter, just as amusing.
http://screencast.com/t/MTk1NjQ4OTEt
It seems when you search for items in the core listing, then delete, the choices don't reset.
#55
I could reproduce this too. You have to reset by selecting the field and remove the content together.
I fixed this by removing every instasearch-has-no-content class, if no value is in the box.
Yes i also don't like the name. Suggestion: instasearch -> instant-search
#56
@dereine #55 works! that error is gone.
looks great.
i'd RTBC this but looks like you want to rename the class.
i wish you could also search titles of collapsed sections, but i understand it's just module names.
#57
#55 appears to fix the problem with the incorrect reset when one highlights and then deletes the content of the search field.
Filtering by Fieldset name: This may introduce more confusion than it helps: e.g. you look for the "pathauto" module, but instead you get the entire fieldset "Pathauto" that may contain many modules. So you are not really narrowing down to what you were searching for, you get the entire fieldset. Especially bad this might be with searching for "Core" or "Übercart", for this search would be very fuzzy...
#58
+++ misc/instasearch.js@@ -0,0 +1,116 @@
+Drupal.instasearch = function (container) {
This still needs to be renamed. We do not abbreviate names in Drupal.
Additionally, even when removing the abbreviation, the name provides absolutely no clue about what this is for.
It seems like this is less about searching, but about filtering, filtering DOM elements, to be more concrete.
+++ misc/instasearch.js@@ -0,0 +1,116 @@
+ var filter = function () {
At least this function should be publicly accessible (and replacable). Changing the entire object/method structure of this thingy wouldn't hurt either though.
+++ modules/system/system.admin.inc@@ -791,6 +792,24 @@ function system_modules($form, $form_state = array()) {
+ $form['#attributes']['class'] = 'instasearch-container';
What if I want more than one filter on my page?
This needs to be an ID and the JS should be passed JS settings to invoke the filtering for a certain ID.
+++ modules/system/system.admin.inc@@ -791,6 +792,24 @@ function system_modules($form, $form_state = array()) {
+ $form['#attached_js']['misc/instasearch.js'] = array();
Obsolete.
I'm on crack. Are you, too?
#59
Update:
- Rename instasearch to instant-search and instant_search
- Removed + $form['#attached_js']['misc/instasearch.js'] = array(); Because this could cause double calls to drupal_add_js
I'm not sure about the last
#60
Reset status.
#61
There was another approach in #39, which had a much shorter patch. Maybe you could give it a review, too.
I did not reroll this one because it did not render the search box in a nice way.
#62
"instaSearch" was supposed to be a proper name, like "Drupal". Not an abbreviation. Anyway, we also don't use underscores in JavaScript function names but camelCase. Thanks for helping land this patch!
#63
@dereine, did you attach the right patch in #59? Looks like the changes you mentioned weren't actually changed.
#64
Updated to resolve patch error
#65
kkaefer: you missed to add --no-prefix to git diff
#66
Sorry for jumping in lately, that's my thing... Just haven't got the time (which is going to change soon luckily).
Couldn't we think of this more like a table filter behavior? The input filter field would be attached to a table (in this case the modules table) by just adding a class 'tablefilter' to the table. Imagine having this filter option on the permission page!!! It's just a few steps away!
The patch I placed @#39 provides most of that functionality.
#67
@skilip: #229193: Incremental filter for permissions page
#68
#69
The last submitted patch failed testing.
#70
@kkaefer: about the permission page patch: that's awesome! About the patch here: I would really prefer having the input textfield being placed by JavaScript instead of by FAPI. Since there's no use for degradability, the input field can be omitted when js is turned off.
#71
skilip: Well if so, it has to be done better than in the patch of #39. I tried that out, and the input field got rendered in the middle of nowhere... (strange positioning inbetween some other div elements).
#72
Updated kkaefers patch and removed some errors with a/ and b/ folders in Git that broke it.
@dereine: is there anything apart from the naming (instasearch or instant-search...) keeping this from being ready?
@Sun: you had quite some concerns. Apart from the naming - maybe you could reroll it and integrate it?
I wonder if this needs a lot of polish, since I guess we will not reuse it a lot, apart from some other tables in Drupal?
#73
Could name it elementFilter.
#74
Just to reiterate yoroy's point about CTRL-F not working.
On the modules page, something like 'token' can appear anything up to 20-30 times because we show module dependencies with 'requires' and 'required by'. Same for i18n (which has dependencies in both directions), this makes CTRL-F completely useless for finding anything on the page. In some cases I've resorted to remembering fragments of text in the module description then using those - more recently, while I'll often still browse the the modules page out of force of habit, even when I'm there, it's almost always quick to alt-TAB to terminal than use drush than to use CTRL-F.
#75
@catch Check out Module Filter for a Drupal 6 solution for finding modules fast. It is clearly not as neat and tidy as the work being done here, but it does meet a general need in Drupal 6 for now.
Will this functionality be available on the permissions page? The same functionality could be useful on the admin blocks page as well.
#76
Hopefully this one passes testing. The patch merges both kkaefer's patch and mine. I removed indexing the data, since my browses crashed during the indexing process. I really think 'tablefilter' is a better name than 'instasearch', since the filtering only applies to tables. Let me know if I forgot something or left out any indispensable functionality of kkaefer's patch.
#77
The last submitted patch failed testing.
#78
Rerolled, hopefully fixing last test failures.
#79
The last submitted patch failed testing.
#80
Bummer!!
#81
Good point. While this patch here doesn't necessarily need to make itself available on the permissions page as well, it does need to implement incremental filters in a way that can be easily used elsewhere, or else we'll risk a repeat of the Dashboard patch (except with incremental filter instead of drag and drop).
#82
@mcrittenden: #229193: Incremental filter for permissions page
#83
@mcrittenden: the filter can be applied to all tables, or groupings of tables.
#84
Nice!
#85
Can just review the function: works just as well as Konstantins patch.
would like a Review from the cody persons.
Sun? dereine?
#86
This new functionality is great - major improvement!
Just a couple of things that weren't that clear to me:
+++ misc/instasearch.js 24 Nov 2009 22:06:28 -0000@@ -0,0 +1,116 @@
+Drupal.instasearch = function (container) {
what's "instasearch"? "instant search"? I thought we never abbreviated variable names.
+++ misc/instasearch.js 24 Nov 2009 22:06:28 -0000@@ -0,0 +1,116 @@
+ if (elem.childNodes[i].nodeType != 8)
+ ret += elem.childNodes[i].nodeType != 1 ?
+ elem.childNodes[i].nodeValue :
Not entirely sure what nodeType is and what the significance of nodeType = 8 is. Maybe a comment should be added here.
I'm on crack. Are you, too?
#87
@stella: which patch did you review?
#88
Crap, yeah, I reviewed the patch from #72. This one makes waaay more sense :) Couldn't really find anything wrong with it - just minor typos and grammar errors in the comments.
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ // Create a reference and define it's variables.
just a grammar thing, here and elsewhere, it should be "its" rather than "it's"
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ * FIlteres all affected table rows for the given string.
Just a typo - should be "Filters"
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+
There's trailing spaces on some lines.
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ // Create a unique idetifier for the new tablefilter instance.
typo
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ window.table_filter_index = (window.table_filter_index == undefined ? 0 : window.table_filter_index + 1);
Should only use one space between ? and 0.
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ // Specify which element is the container of the table(s) on which to applie the behavior.
typo "applie"
This review is powered by Dreditor.
#89
thank you stella. Needs work for last touchups then.
#90
@stella: Thanks for reviewing!
#91
The last submitted patch failed testing.
#92
Can anyone explain to me why this patch failed on 'ForumTestCase'?
#93
HEAD is broken.
#94
yched requested that failed test be re-tested.
#95
I am going to be bold and mark this RTBC, it seems stella given it a review - kkaefer proofed this code was applicable to the permission page filter and dereine chimed in. Skilip finished the work, so this seems good to go.
#96
container[0].tagName.toLowerCase() == 'form'Please use something like
container.is('form')instead of relying on the DOM....attachEventListeners = function() {Do we really need a separate function for two lines of code that is only called in one location?
Drupal.tablefilters[settings.id] = new Drupal.tableFilter(container, settings);Drupal.tablefilters[settings.id].init();Why do we have to call init separately instead of calling it in the constructor?
prototypebut extend the object withthis.ref?Drupal.tableDrag.prototype.restripeTable). We should refactor that code into a more generic function and reuse it instead of duplicating it.#97
+++ includes/theme.inc 25 Nov 2009 08:45:32 -0000@@ -1659,6 +1659,11 @@ function theme_table($variables) {
+ // Attach tablefilter behavior, if required.
+ if (isset($attributes['class']) && in_array('tablefilter', $attributes['class'])) {
+ drupal_add_js('misc/tablefilter.js');
+ }
+++ modules/system/system.admin.inc 25 Nov 2009 08:45:33 -0000
@@ -791,7 +792,15 @@ function system_modules($form, $form_sta
+ $form['#attached'] = array(
+ 'js' => array(
+ array(
+ 'data' => array('tablefilter' => array('system-modules' => array('label' => t('Filter modules'), 'description' => t('Enter the name of a module to filter the list.')))),
+ 'type' => 'setting'
+ ),
+ ),
+ );
uh oh. We want to introduce a drupal_add_tablefilter() function instead, which takes a named array of settings:
$form['#attached']['drupal_add_tablefilter'][] = array('label' => t('Filter modules),
'description' => t('...'),
);
and adds the required JavaScript along with the JS settings.
This function should optionally support a (forced) 'id', but by default, it should take over the #id from the element it is attached to.
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ // Create a reference and define its variables.
This can be removed.
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ var ref = this;
This should be
var self = this;+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ ref.textfield;
Weird. What's the purpose of this?
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ ref.init = function() {
(and elsewhere) Missing space after "function".
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ if (ref.textfield[0]) {
What's that condition?
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ * Filteres all affected table rows for the given string.
+ * All table row containing the string will stay visible, while other rows are hidden.
Typo in "Filteres" and missing blank line after summary. Description is not wrapping at 80 chars.
+++ misc/tablefilter.js 25 Nov 2009 08:45:32 -0000@@ -0,0 +1,180 @@
+ // Walk through each tbody inside the container element.
+ container.find('tbody').each(function() {
Not sure why we need to limit this to tables, but we can investigate in a follow-up.
I'm on crack. Are you, too?
#98
Additionally....
Did anyone consider http://rikrikrik.com/jquery/quicksearch ?
Compared to the code here, that looks quite advanced + awesome.
#99
Not sure about quicksearch. IMO, the code doesn't look "advanced" and has some weird constructs in them. E.g., it relies heavily on the element structure. It also doesn't seem to allow for filtering lists (or anything except for tables).
#100
Ok, rerolled the patch including sun's and kkaefer's suggestions. Thanks btw!
@kkaefer: Using an generic 'tableRestripe' function would be awesome, but is out of the scope of this patch. I removed the index while my browser completely hangs during the indexing process (Safari Snow Leopard Macbook Pro).
@sun: I'm not sure how to apply a drupal_add_tablefilter function. Could you help me out?
The patch currently is only restricted to tables while I don't see the need (ATM) for building extra functionality which is probably not used in core. Maybe we can write another patch when the need for filtering other DOM elements occurs.
#101
Patch contains the JS only.
#102
Er - I think I remember you speaking about adding the input just by javascript.
As the patch does not change system.admin.inc nor any css file, I guess it is up to that.
Still - I don't see any input field on my Modules page.
As for people opposing to add the input per JS - In this case this probably makes sense because the filter won't work anyway if there is no Javascript.
#103
Sorry :-[
#104
I've tested this behavior on the permission page with 500+ table rows. It's a bit slower but fast enough on my browser. Anyone willing to test it on a Windows machine with IE?
#105
While still being unable to judge the patch from a coder's perspective, it works as well as the others. So to speed this a bit up, I'll set it RTBC.
Only one day to go...
#106
hm this has been sitting in RTBC for quite a while... Will it make it in?
#107
Patience my dear! Core committers have a snapshot of what was RTBC on 1 december, they will get to it eventually.
#108
yoroy ah :)
I thought of something like that.
Hehe Zielfoto
#109
1 problem with this as far as I can see... Type the text 'help' 'permissions' or 'configure' in the filter box and it filters based on those terms in the Operations column. Is that intended behaviour? Seems a little odd to me. Shouldn't it be more selective about what is searched - as in use only the Name and Description columns... though it may be handy to search by version too.
#110
mrfelton well this is not nice, but I think not too bad.
If we want to make it selective, it would have to adress every table exactly, which opposes to the plan of making this a general UI element.
Though in followups this could be adressed. First we got to convince Dries this is a good concept at all. Short video for showing this is in the making ;)
#111
@eigentor: I'm 100% sure this is a good feature! Just needs a little tuning in my eyes.
#112
Updated the patch to keep up with head
#113
Works
#114
As the question to solve is wheter Ctrl+F4 isn't enough for searching, up to the great
Filterbox vs Ctrl+F4 shootout http://vimeo.com/groups/drupalux/videos/8241438
Still the strongest reason for the filter box is not in the video: A not-so-geeky person doesn't even know there is a command like Ctrl+F4, and if they know it, they won't use it that fluently.
#115
If you think this is unnecessary try to find views which half of the world depends on...
#116
Re-test of tablefilter-update.patch from comment #112 was requested by webchick.
#117
Still green
#118
My first patch applied succesfully i can confirm this is working great. Love this thing.
#119
Dries have mercy on this issue :)
#120
Dries as a fellow belgian citizen...
Studying at a belgian university just like you did...
I'm asking you...
commit this... XD
#121
eigentor's screencast is very convincing: http://vimeo.com/groups/drupalux/videos/8241438
This page can get sooo very very long, this solution absolutely beats browser's Control-F on one of the real 'work-horse' admin pages. I very much support this getting in, it's more than a nice-to-have.
#122
I really like this patch a lot, and eigentor's screencast indeed does a good job of covering why it's needed. I think this could be one of those "killer features" in Drupal 7 that makes it physically painful to go back and use 6.x (and I just love those :D).
The one downside is that it includes dependencies in the filtered results, which is odd and unexpected. For example, a search for "Taxonomy" gives me "Forum" (what?). I would've assumed it would search only name and description, since one of the things we're trying to get /away/ from is false triggers on every. single. module. that mentions. views.
Looking at the code, I don't immediately see a good way of excluding portions of the table, short of maybe passing in an "ignore" regex argument to Drupal.tableFilter.prototype.filter(). But I'm not sure how that'd deal with, for example, Coder module adding its own little "Code review" chunk in the table.
Even with the dependencies showing, this is still preferable to cmd+F in that at least the list you're scanning through is greatly reduced, but it'd be even better if it could be limited to name/description only. Setting down to needs review to see if anyone has any bright ideas, or counter-points.
#123
Can always use the jQuery .not() to ignore chunks of text. For example changing
if ($(this).text().toLowerCase().indexOf(string) > -1) {to
if ($(this).children().not('.admin-requirements').text().toLowerCase().indexOf(string) > -1) {in the filter function will make it behave like webchick expects.
Though we shouldn't hard code 'admin-requirements' in the filter function of course. Maybe add an extra 'ignore_element' setting that if present uses the not() function to filter out elements we don't wish to check?
Edit: actually the above kills searching the description as well :( will keep looking.
Edit 2:
Maybe?
$(this).find('tr').each(function () {str_exists = false;
var cloned_row = $( this ).clone().find( ".admin-requirements" ).remove().end();
// Walk through each td inside the current tr element.
$(cloned_row).find('td').each(function () {
// Check whether or not the string exists in the current td.
if ($(cloned_row).text().toLowerCase().indexOf(string) > -1) {
// The string is found.
#124
I'd prefer adding a class to each table drawer which can be ignored. Something like 'tablefilter-ignore'.
#125
My original patch at http://drupal.org/node/396478#comment-2096322 had an ignore feature. Not sure why it got removed during the process.
#126
Oh, and someone still has to explain to me why this was made specific to tables. The original version wasn't specific to tables and could for example be applied to lists as well.
#127
I pointed out to Bojhan in IRC that we now have two contrasting search usability patterns in core. When I do a Ctrl-F search in Firefox, to stop the search and 'reset', I can click the 'X' to get out of the finding. Other places where we have 'filters' we have 'Reset' buttons. For this? User needs to click the filter input box, select the text, and press the Delete key. We should really make sure this filter includes an easier, not-hidden way to 'Reset' the filter. Otherwise I think we might have a lot of support requests for "I typed in a word and now I can't view all my modules again!"
#128
Ok I'll give kkeafer's latest patch (see #72), so it's not table-specific any more, a reroll and try to incorporate all issues/changes after that.
#129
@kkaefer: Could you give us an example (use case) of why we should rebuild this to support item lists? IMHO we shouldn't write extra code which most probably isn't used after all. No need to explain that, I guess.
#130
Actually it's the other way around. Restricting it to table rows requires additional code.
#131
Is that true? Filtering only tables is for obvious reasons faster any given element. While it is faster due to limited elements it doesn't have to be indexed. Again: do we have a use case for having the need of filtering other elements than tables?
#132
Yes, I'm stubborn. Sorry 'bout that :(.
#133
skilip: Sorry but which such limited time its completely useless to discuss this over and over again, if we can make it more abstract there is no doubt we should. The world of contrib should be allowed to filter other things than tables, I really don't see any use to restricting it.
#134
@Bojhan: I didn't reopen the discussion. I'm just trying to avoid more delay.
#135
Ok initial patch. Took longer than I hoped (mostly because of the zebra updating).
I'll continue tomorrow, but I appreciate reviews. Patch is usable.
#136
The last submitted patch, instantfilter.patch, failed testing.
#137
grmbl
#138
I've tried the patch @#137. It is quite faster than the last instantfilter patch I applied. The indexing slowing my browser down was my biggest concern. Actually it works like a charm. I've added the 'help', 'permissions' and 'configure' table drawers to the ignore selectors. Green light!!
#139
The last submitted patch, instantfilter_ 396478#138.patch, failed testing.
#140
Removed hash in filename
#141
The last submitted patch, instantfilter_ 396478_140.patch, failed testing.
#142
Re-test of instantfilter_ 396478_140.patch from comment #140 was requested by Bojhan.
#143
Minor update.
- cleaner form array
- added back name of textfield
#144
Works very smooth, hopefully this baby is finished by the 15th
#145
Not sure if this is still a work in progress but I gave it a little "comment review" nonetheless.
+ // added dynamicly (e.g. using AJAX). If so, the index needs to be rebuild.Typo here.
+/**+ * Get text value of an item.
+ */
+/**+ * Removes the 'no results' message.
+ */
Be consistent with grammar in comments (Remove vs. Removes, etc.)
+ // Otherwise dipslay the 'no results message.Typo here.
+/**+ * Theme an input textfield
+ */
Needs a period.
+/**+ * TODO comments.
+ */
Obviously waiting on comments.
#146
Putting back to needs review apart from those comment reviews, what about the actual code?
#147
The filter does not work anymore for me in Firefox 3.5 (neither #137 nor #143). Working fine in IE8. Please someone cross-check.
#148
+++ misc/instantfilter.js 1 Jan 1970 00:00:00 -0000@@ -0,0 +1,267 @@
+ if (!container.hasClass('.instantfilter-processed')) {
+ container
+ .addClass('instantfilter-processed')
use .once()
+++ misc/instantfilter.js 1 Jan 1970 00:00:00 -0000@@ -0,0 +1,267 @@
+/**
+ * The tableFilter object.
+ */
+Drupal.instantFilter = function (container, settings) {
Looks like an instantFilter, not tableFilter
+++ misc/instantfilter.js 1 Jan 1970 00:00:00 -0000@@ -0,0 +1,267 @@
+
+/**
+ * Get text value of an item.
+ */
+Drupal.instantFilter.prototype.getText = function (element) {
How is this different from $().text() ?
Other than that (and the comments above), the screencast looks good and the code looks good too.
#149
The code is good. But we need comments.
#150
I really didn't liked that this script was creating the form element. At first I tried to re-use states.js. This isn't however possible as states.js won't allow to compare values of dependants to dependees. It would take to many alterations to states.js to make it work. States.js could be more powerfull, but that'll be D8.
This patch is based upon the previous one of #143. The filter input element is now created server side. Multiple groups are allowed.
#151
The last submitted patch, instantfilter1a.patch, failed testing.
#152
#153
#154
I haven't tested your patch yet, but the reason for adding the input field client side, is that all filtering is done client side as well. When the user disables Javascript, the input field makes no sense anymore. Unless of course you add server side handling for the filter, but that's for obvious reasons out of the scope of this patch.
#155
subscribing
#156
I've played around with it, and like it a lot. My initial thoughts?
.js-show {display: none;
}
html.js .js-show {
display: inline;
}
#157
Might be nice on the permissions page.
#158
I've already tested this on the permissions page and it works like a charm. GET THIS IN!! :P
#159
permissions page is a little bit tricky as groups are siblings of items (same table). theme_table however doesn't support multiple tbody elements (which is valid). I already created a patch #31535: Grouping of rows in theme_table() for that but it requires a api changes so it is pushed to D8; maybe worth an exception?
#160
Sorry, but it feels absolutely way too late for this now. :( What a shame...
Can someone pull this functionality into a contrib module for D7?
#161
What a shame indeed. We had this RTBC before code freeze.
#162
Well. RTBC only counts if it's actually RTBC, which based on recent comments, it was not. :(
Feature freeze was 9/1, alpha 1 was 1/15, and this didn't make either of those deadlines, nor alpha2. And from the sound of it, there's a whole hell of a lot of work left to do on it, plus follow-up to get it showing up other places in core. We need people focused on critical bugs now.
Sucks, too, cos I really love this patch. :(
#163
You're absolutely right. Don't mind me, just moaning.
#164
Added instantfilter to permissions page. Therefor I merged patch from #31535: Grouping of rows in theme_table() into this one. Just so you could try it out...
#165
Please coordinate with me + http://drupal.org/project/admin_menu re: contrib.
#166
@sun Honestly this doesn't belong in admin_menu.
#167
I created a project: http://drupal.org/project/instantfilter
#168
@casey: Will you mention the guys who helped here on your project page?
#169
You are absolutely right! I updated the module page.
#170
reminding of Module filter - http://drupal.org/project/module_filter
just in case it's of any help for this patch
#171
Downgrading all D8 criticals to major per http://drupal.org/node/45111
#172
sub
#173
#174
The last submitted patch, instantfilter1c.patch, failed testing.
#175
Please provide rationale when changing priority :( We want to improve site builder tools for D8 and one of the big issues of the modules page is people having a hard time finding the thing they just installed.
#176
Totally an accident. I just meant to retest the patch (needs a reroll). I blame gremlins. :)
#177
...subscribing.
#178
sub
#179
Hm this is such a no-brainer that maybe we do not even care working on it.
From an UX standpoint the last solutions worked: Filter only filters by module name. Any time I use Firefox's In-Page-Search I swear a lot because there may be 20-30 occurrencies of the name "Media" when I just want to get to the media module.
Any Dev willing to reroll this one to the current state of core?
#180
The rerole was less hard from what you could expect from the rej.
Tested manually the module and permission page. Take sure to clear the cache.
#181
The last submitted patch, instantfilter1c.patch, failed testing.
#182
Addapting the tests, let's have a look.
#183
The last submitted patch, instantfilter1c_0.patch, failed testing.
#184
Okay here is one which doesn't drop permissions
#185
Have tested the patch. It applys all right ;)
But it appears to be also searching through the module description.
It should only take into account the module names IMHO. I read through the thread to find if there is a consensus about that, but am not sure if there is a consensus on that. So UX people, please give your opinion.
#186
Yhea the consensus is only on project names.
#187
There may be cases where including the module description might be required/useful. How about a "Also include module description in the filter search" checkbox? Having it unchecked by default serves the general consensus of not including descriptions during search, but still allows those that require it to use it. This checkbox's last state should be remembered though.
#188
It makes absolute sense to not filter by the module description.
#189
The project page mentioned in #167 is being actively developed (mostly by me). Some changes are performance improvements, some are targeted towards more flexibility. Anyway, it would be nice to coordinate. Some changes might not be core ready or even worthy, but some might. Only way to find out is by getting some input. :)
My goal for the project is making an already great end solution, a flexible search library for multiple admin pages.
#190
@Kiphaas7 Could you reveiw the patch? Would love to get your knowledge about instant filter, to be combined into a fix in drupal 8 core.
#191
@kiphaas this is great. I could see many places where this is useful. The list of all content (admin/content/node) or the list of all users.
Do you see a way to filter across pagination, or a way to work around the pagination? Cause this would be really awesome...
#192
@190:
I could, but I believe the original git commit of the instant filter project is the same as this one (sans the 'do not search in descriptions', but thats minor, and changes to the js are rather limited. My commits are either minor performance improvements, or API additions, which could be handled in followup issues (especially the API additions need some bikeshedding).
@191:
Instant filter works based on content present in the html. So, no. The only way it would work would be to have some sort of JS pagination, or feed in all results via an ajax request to instant filter. Anyway, it always boils down to having all the results available. Pagination avoids querying the database for all results, because hundreds or thousands of results hurt performance. Doing it anyway for Instant filter would negate the whole point of pagination.
Actually, I would argue that pages where pagination isn't strictly necessary for performance, the choices for cleaning up a page would be:
- use drupal's php pagination (no instant filter)
- use instant filter (no php pagination)
- use instant filter, while limiting the number of results on the page. This is already easily possible with the instant filter project.
Option 3 however, comes dangerously close to re-inventing the wheel. I believe it was casey who pointed me towards the following page:
http://blog.jqueryui.com/2011/02/unleash-the-grid/
Also a very viable option, considering the jQuery UI integration since D7!
#193
I think searching on project description is perfectly valid. For instance if I was new to Drupal and wanted to enable the translation module searching for 'language' should return the content translation module. Similarly 'navigation' should return the menu module. I'm sure there are many more examples where a valid keyword appears in the description only.
Who is really going to search for 'allows' or other common words? If searching on descriptions impacts performance, maybe take it out, but removing it because it returns too many results on a common word.... really?
#194
@meatsack You drive a interesting point, however we have to account for two use cases both the "I know what I am looking for" and "I am not sure how its called". The first one is far more prominent and its unlikely we can really accommodate "I am not sure how its called" greatly even if we incorporate the descriptions (for loads of modules your examples don't apply).
#195
If someone knows that they want to do something with RSS feeds, they won't search for "Aggregator". They are more likely to search "RSS" or "feeds". Filtering the description is one of the things that makes this patch useful.
#196
@RobLoach I am not sure, descriptions are quite useful - but if we really want to accommodate the use cases you guys mention than we need to reevaluate these descriptions so they all have meaningful keywords. However I don't want this description search to become an excuse for poorly labeled modules in core at least :)
#197
I tested the filtering by which also inluded the description and that works quite ok as it still narrows down the search results a lot.
In comparison at the moment I often use the browser's on site search, and man, that is hell. Because it also finds every single mention of say flag module in all the modules that it depends on.
So it should not be that different in usage.
Having a checkbox solution to toggle the search by description could be a good solution, of course this brings up if it should be on or off by default.
It could be "Search also by module description" that could be checked by default.
#198
"We should have it", "No we shouldn't"... that's why I proposed to make this optional & remembered back in #187. That would satisfy both parties.
#199
...btw, I've been looking for an easy way to filter down to all enabled modules that require/use chaos tools in order to troubleshoot #523712: Call-time pass-by-reference deprecated in ctools_plugin_api_include(), line 156 of ../ctools/includes/plugins.inc. No way to do this right now, so I'd also love to have the option to include the requirements section while filtering.
#200
My personal oppinion about getting more and more features in ... keep it simple in core and extend it in contrib if you really want all this features. I know this is an issue about the UX of a central place which many users will use, but i would say that 90% is already covered by having the modulename filterable.
A normal user for example will not care that much about getting all modules which require ctools.
#201
I am totally in agreement with @dereine here, it looks like by filtering module name and description we should tackle the 80/90% use case. Contrib modules that already exist can tackle the other 10%, which is more advanced.
@klonos It is a good lesson in UX there are often trade offs when you add "just one" extra checkbox. The form becomes less easy to scan and there is now an expectation that this search box can have states. Satisfying both parties is technically easy, but causes problems for the end-user. Often when we make the decision, not to make a decision and just add a checkbox all we are doing is causing additional problems for the 80% of our users, to satisfy a need of very few.
I am going to give this a visual review later today, can anyone else review the code?
#202
@200:
I completely agree, and that is what I tried to do with the instant filter project: make it extensible and nice to other js files. I intend to add extra functionality via submodules. Great way to test the extensibility of the instantfilter.js file too!
http://drupal.org/project/issues/instantfilter?text=&status=All&prioriti...
http://drupal.org/node/793448/commits
#203
@dereine:
Yes they would Daniel. If they actually want to disable it but they need to disable all modules that require it first. Do you really believe this is a far-fetched case? I don't. Unless of course we manage to have in core some way to batch-disable modules with dependencies that might have more dependencies in turn and so on... (the way we do when we batch-enable module dependencies).
@Bojhan: I understand that your job title is Usability Expert so this is what you do, but...
Where did you actually get these 80%-20% or 90%-10% stats. Is this official or just a personal guess/estimation. Anyways...
"Contrib modules that already exist"??? Care to point to one that actually does this? I hope I make myself clear here: I am not speaking for myself. I can manage pretty fine the way the modules page is right now (without the filter), but I've been a drupal user for more than 2 years now. How about newcomers? Do we simply want to say "Oh, well, what can you do? Someone will eventually come with a contrib module that does what you want. Someday..."* or "Ah yes! Here's this contrib module that does what you are after."
*(basically this is what I feel we generally do to newbies in d.o. ...despite the fact that most of their requests are valid usability problems)
ok this seems fair enough, but ...still not convinced (lack of official usability study/stats and all). Even if so, I'd really love to see some actual, existing contrib project that covers the needs of this "10% of advanced audience" so we can point people to it and actually provide them with a solution.
Till this is *actually* done in contrib, my (not-so) humble opinion is that this needs to be addressed in core. I mean, unless of course there is *actual* proof that it either noticeably decreases performance or hinders UX. OTOH I'm afraid this is going to be a "core needs to be kept simple" thing so I believe I'm crying to a wall here.
#204
...Unless of course Kiphaas7 "jumps to the rescue" and actually implements (the just-created by me) #1294506: Allow optional (by checkbox) filtering of the description/requirements section. in his Module filter NG (fork of the Module Filter project) that he eventually intents to merge in Instant filter.
...in which case I'll just shut up ;)
PS:
#205
It should be solved in contrib, and it probably will be solved there ;).
back ontopic: if I have some time tonight, I'll review the javascript file of the patch, and compare it with the working version of the instant filter project js file.
#206
I completely agree, and that is what I tried to do with the instant filter project: make it extensible and nice to other js files. I intend to add extra functionality via submodules. Great way to test the extensibility of the instantfilter.js file too!As the code is OOP it should be somehow possible to extend the behaviour in a contrib module, but i'm not 100% sure.
Where did you actually get these 80%-20% or 90%-10% stats. Is this official or just a personal guess/estimation. Anyways...I guess ;) this is just personal guess, but it's quite often a very good one. Additional making the UI simple also helps the more advanced user, as it improves the feeling most time.
In general i recommend http://london2011.drupal.org/coreconversation/search-drupal-8-search-too... :)
The summary here is: "let contrib do it, because it can do it much better.".
#207
@klonos Your objections are really understandable, but this does indeed fall in the realm of keeping core simple.
When we talk about 80-20 or 90-10 stats we are in most cases going from our experience talking to and observing users (we have done multiple tests on the module screen) however that did not generate a statstical value on which we can make decisions for new functionality. I find it quite unrealistic to assume, that we can provide statistical data for each new change to core (our resources are increadibly limited). With new functionality it is always a hard decision on which functionality to include and which not.
When we say contributed modules can tackle this use case, we are allowing for them to be developed for Drupal 8. Short term something like an extension to Kiphaas his module should be fine.
#208
#206:
Yes, one could override the prototyped properties of the instant filter object. But, that would mean a lot of code duplication. Invoking weighted hooks (as drupal already does serverside, so it's not a new concept) just to alter/add/delete an attribute of an item in the cache or search result is just a few lines extra.
plus, this allows incremental additions as well. Multiple modules wanting to alter instantfilter with the current code probably leads to .prototype.hell ;).
#209
@klonos: "80/20" is a design rule derived from the Pareto principle (and as bojhan points out, backed by actual user observations in this case).
Correct me if I'm wrong here, but the way you are discussing this now looks like you're opposed to fixing the 80% use case because contrib hasn't dealt with the 20% use case yet, and thus we didn't think of the newcomers? But especially newcomers would fall right into that 80%, so I don't really understand your argument there.
#210
+++ b/includes/theme.incundefined@@ -1705,46 +1746,58 @@ function theme_table($variables) {
+ foreach ($rows_by_groups as $rowgroup_id => $rows) {
+ if (count($rows)) {
This identation looks kind of weird.
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ this.settings = $.extend({
It would be kind of cool if there would be some kind of documentation what this does.
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ this.container = $(document.body);
Not sure whether it's worth to change now, but i think drupal uses this.$foo if it's a jQuery object.
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ self.applyFilter($.trim(this.element.val().toLowerCase()));
Isn't it possible to use "this" here?
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ * Get text value of an item.
Some notes about the recursion would be cool as well.
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ for (var selector in this.settings.items) {
Let's use a jQuery.each with another function on the object itself + jQuery.proxy
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ this.container.find(selector).each(function () {
+ var item = $.extend({}, self.settings.items[selector], {
+ element: $(this),
+ value: self.getValue(this, self.settings.items[selector]),
+ groups: []
+ });
+
+ self.items[i] = item;
+ item.element.data('instantfilter:' + self.instanceID + ':item', i);
+ i++;
This should be another function on the object from my perspective.
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ for (var selector in this.settings.groups) {
+ this.container.find(selector).each(function () {
+ var group = $.extend({}, self.settings.groups[selector], {
Same as above
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ this.search = search;
Wouldn't something like this.currentSearch explains a bit better what it does?
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ var match = item.value.indexOf(this.search) >= 0;
This line should be documentation. For some people it's probably not clear that indexOf actually searches whether the string matches.
+++ b/misc/instantfilter.jsundefined@@ -0,0 +1,216 @@
+ if (this.results) {
+ this.element.closest('.form-item').find('.instantfilter-no-results').remove();
+ }
+ else {
+ if (!this.element.closest('.form-item').find('.instantfilter-no-results').length) {
+ this.element.closest('.form-item').append($('<p class="instantfilter-no-results"/>').text(this.settings.empty));
+ };
I think it would make sense to store the location of the no-result text, but this is perhaps something for a follow up.
Doesn't it make sense to use Drupal.theme here?
+++ b/modules/system/system.admin.incundefined@@ -783,6 +783,30 @@ function system_modules($form, $form_state = array()) {
+ $form['modulesfilter']['#attached']['library'][] = array('system', 'instantfilter');
This should better be modules_filter or something like this, as this long word is hard to parse and quite confusing. This problem is there for all places.
+++ b/modules/system/system.admin.incundefined@@ -2581,7 +2605,7 @@ function theme_system_modules_fieldset($variables) {
+ $row[] = array('data' => drupal_render($module['version']), 'class' => 'version');
Shouldn't class be an array in d8/d7 and the single key is just a workaround? Does someone know this?
-1 days to next Drupal core point release.
#211
@Dries http://www.theatlantic.com/technology/archive/2011/08/crazy-90-percent-o...
#212
There are a couple of UX problems, we need to enable highlighting (in a non-jarring way, I think dmitrig had some parts of that in earlier patches on the permission issue too), like in http://drupal.org/files/issues/Snapshot%202008-03-19%2000-19-47.png.
Also lets move the filter to the right use html5 placeholder "Filter modules" or something like that to make the description go away.
#213
Also noticed, when you search the permission page - the table header changes (probably the lacking descriptions?) anyways that makes it look a bit wonky.
#214
Sorry for the late reply, I was away.
Anyways, as I said previously, I am not talking about my needs here, since I've been using Drupal for more than 2 years now. In this time I've read my share of literature and I know how to handle things + I know how to use ctrl+f ;) I am more playing the role of the devil's advocate here, trying to think as a newbie. When I ask my lady or my customers to do stuff in the Drupal UI, they seem puzzled quite often and when I explain to them how things work, I get a lot of "Why not simply have it this/that way?" feedback. So, what I do is transform most of this feedback to filed issues in the queue in d.o.
Basically, what follows is more like a generic rant - still related to this issue, but doesn't actually help solving it. So, feel free to read only if you spare the time...
@yoroy, #209:
Actually, you are wrong Roy, so I'm correcting: I didn't say "Don't fix this issue *at all*" (and thus "Don't fix -the supposedly- 80% of use cases"). I was more like "Why not also include *optional* features that will also satisfy the rest of the -again, supposedly- 20%?".
Yep you are right there, but this observation (or more like a general d.o feeling based on personal experience) wasn't focused on this issue alone. As I said back in #203, I feel that in most such cases, we simply drive newbies away by saying: "Oh, well, what can you do? Someone will eventually come with a contrib module that does what you want. Someday...", while we should be more like: "Ah yes! Here's this contrib module (or workaround) that does what you are after.".
...point being that we should be providing people with actual solutions/workarounds (when we can) instead of asking them to hope & pray for a solution in the future. What is most disturbing with this behavior of ours is not my feeling that some people (mainly project maintainers) have this tendency to quickly close issues, rather than the fact that some of them also have the confidence that they actually helped the person asking for help, when -for example- they simply mark the issue as "fixed" instead of "postponed" or "closed (won't fix)". They need to understand that "fixed" means precisely that!! - not "will soon/someday be fixed ...by someone" or "closed because I don't like open issues in my queue".
Please read my comment in #203 more thoroughly if still unclear.
#215
#211
Useless number unless the audience of those studies (I couldn't find a source) is exactly (or at least roughly) the same as the audience using the drupal modules page. Probably the number will be less spectacular then.
#216
...you have a point there, but still that case teaches us to not take our knowledge of things as granted for everyone else using our products.
Somehow related: My old boss used to say to me (when I was writing documentation, but still applies to UI/UX too):
"It should be that simple, that if I pulled a granny from the street and asked her to do it..." (by reading through my documentation and following along) "...she should have no trouble at all doing it."
#217
I'm a bit confused here by klonos's comments here. If you have a particular use case to be able to filter the module list down to what modules require a certain module (which, it should be stated, you also can't do with the current modules page), that seems best done in a separate feature request issue. Let's keep this one to being able to search on title/description so it doesn't become a 400 reply issue instead of a 200 reply issue. :)
Though I will say that what you're asking for seems a lot more like more something that would go into a tool like Devel module than in the core of a CMS aimed at non-programmers.
#218
Here is a new patch with the changes mentioned in http://drupal.org/node/396478#comment-5054382
Manual testing worked as expected.
#219
http://drupal.org/node/396478#comment-5074824 is required
#220
It's not possible to highlight a certain text in html without some extra effort. There is a jquery plugin which does this: https://github.com/bartaz/sandbox.js/blob/master/jquery.highlight.js but sadly it's not licensed under GPL. I contacted the author so let's see whether there is a chance for it. Alternative we could write a seperate jquery plugin for it.
#221
Sorry, but I see 3 different and separate issues here, which all need their own attention in detail:
1) Enhancement of themed tables to contain a concept of "rowgroups" (which is yet unknown to me). That's a wide-ranging and pretty fundamental change on its own.
2) Introduction of a JS library (+ Drupal integration helpers) that's able to quickly filter a table. The implementation in this patch is most probably one of the slowest I can think of. An implementation that holds what it promises needs to be able to filter 1,000+ rows instantly. I've written a couple of these myself and know that this requires a range of performance optimization tricks, which is a discussion that definitely does not fit into this issue. I'm all in for getting a JS-based instant/quick-filter into core, but let's do that properly.
3) Making the modules page searchable/filterable (which is a nice idea, but IMO, a poor answer for the actual problem space at hand -- however, if the usability team thinks this makes sense, then I couldn't care less, and given 1 + 2 as resolved, the actually required changes remaining for this should be minimal.)
All of these also require discussion, solid and dedicated tests, and partially manual testing, so my recommendation would be to spin off 1 + 2 into separate issues first, and postpone this one (3) on the other two.
Can someone create these two issues and link to them here?
(However, please check for existing issues first. I'm half-way sure that I've seen at least an issue about a instant/quick filter in the past...)
Lastly, this patch somehow contains changes to the user permissions table, which absolutely need to be removed and discussed/tested separately. Read #1203766: With large number of permissions /admin/people/permissions becomes unusable to learn more.
#222
sun:
Coud you post your experiences on building a performant filtering solution in the issue queue of instant filter? I'd love to patch up that project with your experience. The project is a direct derivative of the patches in this issue.
#223
#31535: Grouping of rows in theme_table()
#224
IMHO, this issue's title (and scope) should change to implement a generic filter API that can be used in other pages besides the modules page. The contrib project Instant filter does that and it does it very well, so we shouldn't go re-inventing the wheel.
PS: this is D8 now, so changing tag...
#225
Would #229193: Incremental filter for permissions page be good to work on the specific JS library?
#226
For 1. in #221: #31535: Grouping of rows in theme_table()
#227
Note that if we're considering mobile at all, then we should probably make sure this works without JavaScript first, then worry about having it insta-search-ify-on-demand.com.
#228
Updated the issue summary. And the tags.
#229
I would sooner attempt to use drush over ssh on a mobile device than navigate any typical website's module page on a typical mobile device. I dare say we should probably redesign the whole modules user interface (again) if we are considering it for mobile. If we are exposing an API for JS filtering/searching then yes it needs to be mobile friendly.
#230
A related subset issue that uses a php based filter(instead of js) and only use project name: #602528: Filter module page on project name.
#231
I think we need a js based filter, and it should search on three things for certain:
- module name
- module description
- project name
There are also some other things we *could* search on:
- new as in #1355526: Add a way to determine the date a module was added so the modules page can use it for sort
- tags as mentioned briefly in #1355292: Come up with better alternatives for groupings on the modules page
#232
FYI: Related feature request: #1357662: Collaborative module categorisation and search relevance improvements
#233
Per @sun's comment
1) #31535: Grouping of rows in theme_table()
2) Still needs to be created
3) #396478: Searchable modules page
This issue will remain postponed till 1,2 are near completion.
#234
After talking a bit with Bojhan on IRC, we came to the following stalemate, which needs some input from javascript gurus in order to help move this issue forward:
In my mind, there are 2 fundamentally different approaches to creating a searchable modules page:
Personally, I'd lean towards the js-only solution. Not having a non-js fallback is not a problem in my mind (you just get the old situation). It does provide more challenges than the ajax solution, but if done right we also get a nice js search framework as an added bonus. Plus the responsiveness is, again if done properly, much better than the ajax solution.
EDIT: Though I don't have much time these days to work on it, I'd love to be involved in some way in moving this forward.
EDIT2: added accessibility issues to ajax solution as well.
#235
if a non-js fallback should be offered at all. <= no. create the whole shebang out of JS so that the fallback is a nonsearchable modules page. The nineties called and they want their browser back.
#236
I wholeheartedly agree with Kiphaas7 and chx that JS-only is the way to go, for the same reasons they listed.
#237
in favor of JS-only
#238
I'm not against supporting it, but honestly now... who does serious admin work on a regular basis on mobile? Especially enabling/disabling modules.
#239
@klonos Maybe not much on a mobile phone but you'll find more and more people doing it on tablet devices.
#240
I didn't mention the word "phone" in my post.-
Tablets are in the mobile category AFAICT and they are equally not suited for serious admin work IMO. Be it the small screen size or the lack of a proper keyboard/mouse set. I'm not saying that it can't be done or that some people don't do it -not saying that the need to do it never arises either-, but rather trying to point out that targeting the audience doing admin tasks on mobile is wasted time and effort (because they must be a very small percentage of admins - again, I'm not talking about mobile users/visitors of a site). If that sounds a bit harsh, then at the very least we shouldn't postpone features that will ease the job of the rest of the admins (the majority of us if you ask me) based on consideration of people that choose to do admin tasks on mobile. If there is the occasional need to enable/disable a module or two while on a tablet, then I'm sure one can live with the extra effort/scrolling/whatever required in such an environment.
I hope I made myself clear this time.
#241
I have had to do admin tasks on mobile devices, simply because stuff broke, they called me to fix it but my phone was to only device with an internet connection in the vicinity. This issue may simply be labeled paving the cowpaths.
#242
...as I (and others before me) said before, there can indeed be times when there's a need to administer your site from a mobile device. This is an exception to the rule though and it will most likely happen to be required only on emergency situations. Thus, trying to solve this issue while keeping an eye on mobile-using admins only slows down the implementation of a new ground-breaking feature that aims to mend one of Drupal's oldest and most annoying flaws.
#243
In my opinion, having acceptable mobile performance will be there once we get acceptable performance with >1000 items filtering. Nothing more, nothing less. I just made a list with pros and cons, not the official roadmap to an implementation of the new drupal modules search filter :).
#244
Your mobile device does not have JavaScript? What did you buy, a Nokia Communicator? Wait. http://en.wikipedia.org/wiki/Web_Browser_for_S60 even those have JS in them.
#245
[completely-offtopic]Whoohaahaaahaa!! That's the most sensible thing I've heard about JS for UI's in ages! 1+ 4 chx[/completely-offtopic]