Searchable modules page

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".

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.

The last submitted patch failed testing.

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.

screenshots

The last submitted patch failed testing.

Oh my - lets *please* refresh this patch and push it in.

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.

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?

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.

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.

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.

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.

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.

tagging

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.

#229193: Incremental filter for permissions page

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

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.

+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.

http://drupal.org/node/538904 has more general modules page discussion and outlines goals we want to achieve when redesigning it.

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.

This is the initial version of InstaSearch.

There are three types of elements:

• Containers: Each container is a region in which you can search for items. Containers have the class instasearch-container.
• Items: An item is a DOM element that should be displayed or hidden depending on what the user typed into the filter textfield. Each element with the class instasearch-item becomes an item.
• Ignores: If an item contains certain elements that should be ignored when searching (e.g. the “More Help” links in the modules listing), it should have the class instasearch-ignore.

Doesn’t tackle the zebra striping problem.

The search field should probably be formatted in a more visually appealing way.

Awesome patches in the Drupal 7 issue queue!

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.

sun: having a filter like this is part of that revamp

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?

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.

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.

This is UX which should be fine even past slush.

.

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.

sub

Still doesn't detect pastes correctly.

Subscribing
http://drupal.org/node/598738#comment-2132216

The last submitted patch failed testing.

HEAD was broken. Resetting status.

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.

The last submitted patch failed testing.

#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...

added d7ux tag

@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.

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

Can anyone still work on this?

@Bojhan we just gotta get someone to reroll one of the two patches.

Rerolled Konstantins Patch.

It appears to work fine, also when you paste somthing into the search box.

Why not use your browser's search feature?

@Dries: CTRL + F is just a workaround for a usability problem of the module's page.

@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.

Additionally, CTRL-F forces you to tab through false positives that appear "somewhere" in the page - notably hidden in admin_menu's menu tree.

yes, the filtering down is nice and clean - leaves only few results on the page.

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?

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.

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.

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

@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.

#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...

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.

At least this function should be publicly accessible (and replacable). Changing the entire object/method structure of this thingy wouldn't hurt either though.

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.

Obsolete.

I'm on crack. Are you, too?

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

Reset status.

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.

"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!

@dereine, did you attach the right patch in #59? Looks like the changes you mentioned weren't actually changed.

Updated to resolve patch error

kkaefer: you missed to add --no-prefix to git diff

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.

@skilip: #229193: Incremental filter for permissions page

The last submitted patch failed testing.

@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.

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).

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?

Could name it elementFilter.

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.

@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.

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.

The last submitted patch failed testing.

Rerolled, hopefully fixing last test failures.

The last submitted patch failed testing.

Bummer!!

Will this functionality be available on the permissions page?

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).

@mcrittenden: #229193: Incremental filter for permissions page

@mcrittenden: the filter can be applied to all tables, or groupings of tables.

the filter can be applied to all tables, or groupings of tables.

Nice!

Can just review the function: works just as well as Konstantins patch.

would like a Review from the cody persons.

Sun? dereine?

This new functionality is great - major improvement!

Just a couple of things that weren't that clear to me:

what's "instasearch"? "instant search"? I thought we never abbreviated variable names.

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?

@stella: which patch did you review?

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.

just a grammar thing, here and elsewhere, it should be "its" rather than "it's"

Just a typo - should be "Filters"

There's trailing spaces on some lines.

typo

Should only use one space between ? and 0.

typo "applie"

This review is powered by Dreditor.

thank you stella. Needs work for last touchups then.

@stella: Thanks for reviewing!

The last submitted patch failed testing.

Can anyone explain to me why this patch failed on 'ForumTestCase'?

HEAD is broken.

yched requested that failed test be re-tested.

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.

• container[0].tagName.toLowerCase() == 'form'
  
  Please use something like container.is('form') instead of relying on the DOM.
• We should use a JavaScript theme function for that button instead of creating it programatically.
• ...attachEventListeners = function() {
  
  Do we really need a separate function for two lines of code that is only called in one location?
• Why is the patch now restricted to filtering tables only? The previous version didn’t have that restriction and could for example be used for lists as well.
• Why did you remove the index? Did you conduct performance tests without the index? The previous version probably works a lot faster with many elements.
• Since the text field is programatically generated, it’s harder to impossible to customize the test for various locations.
• 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?
• Is there a reason why we don’t use prototype but extend the object with this.ref?
• There’s zebraing code in tabledrag.js already (Drupal.tableDrag.prototype.restripeTable). We should refactor that code into a more generic function and reuse it instead of duplicating it.

uh oh. We want to introduce a drupal_add_tablefilter() function instead, which takes a named array of settings:

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.

This can be removed.

This should be

Weird. What's the purpose of this?

(and elsewhere) Missing space after "function".

What's that condition?

Typo in "Filteres" and missing blank line after summary. Description is not wrapping at 80 chars.

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?

Additionally....

Did anyone consider http://rikrikrik.com/jquery/quicksearch ?

Compared to the code here, that looks quite advanced + awesome.

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).

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.

Patch contains the JS only.

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.

Sorry :-[

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?

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...

hm this has been sitting in RTBC for quite a while... Will it make it in?

Patience my dear! Core committers have a snapshot of what was RTBC on 1 december, they will get to it eventually.

yoroy ah :)
I thought of something like that.
Hehe Zielfoto

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.

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 ;)

@eigentor: I'm 100% sure this is a good feature! Just needs a little tuning in my eyes.

Updated the patch to keep up with head

Works

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.

If you think this is unnecessary try to find views which half of the world depends on...

Re-test of tablefilter-update.patch from comment #112 was requested by webchick.

Still green

My first patch applied succesfully i can confirm this is working great. Love this thing.