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: Allow table row groups in table.html.twig and template_preprocess_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.

CommentFileSizeAuthor
#218 396478-instafilter.patch20.97 KBdawehner
#188 396478-instafilter.patch20.26 KBdawehner
#184 instantfilter1c.patch20.24 KBdawehner
#182 instantfilter1c_0.patch16.74 KBdawehner
#180 instantfilter1c.patch20.25 KBdawehner
#164 instantfilter1c.patch20.45 KBcasey
#152 instantfilter1a.patch9.25 KBcasey
#150 instantfilter1a.patch9.26 KBcasey
#143 instantfilter.patch10.88 KBcasey
#140 instantfilter_ 396478_140.patch9.87 KBskilip
#138 instantfilter_ 396478#138.patch9.87 KBskilip
#137 instantfilter.patch9.83 KBcasey
#135 instantfilter.patch9.83 KBcasey
#112 tablefilter-update.patch9.39 KBeigentor
#103 tablefilter-396478-7.patch9.44 KBskilip
#100 tablefilter-396478-6.patch6.27 KBskilip
#90 tablefilter-396478-5.patch8.72 KBskilip
#80 tablefilter-396478-4.patch8.72 KBskilip
#78 tablefilter-396478-3.patch8.72 KBskilip
#76 tablefilter-396478-2.patch8.69 KBskilip
#72 instasearch-new.patch5.99 KBeigentor
#64 instasearch.patch5.73 KBkkaefer
#59 searchable.patch5.63 KBdawehner
#55 searchable.patch5.63 KBdawehner
#46 searchable.patch5.85 KBeigentor
#39 tablefilter.js_.txt1018 bytesskilip
#39 tablefilter-598738-1.patch1.31 KBskilip
#35 7d894ab1.patch6.14 KBkkaefer
#23 64e21cc4.patch4.31 KBkkaefer
#12 vt_modules_search_frh.patch11.97 KBFrando
#5 modulesearch3.png137.92 KBFrando
#5 modulesearch4.png125.23 KBFrando
#5 modulesearch5.png113.07 KBFrando
#4 vt_modules_search_frh.patch11.7 KBFrando
#4 vt_modules_search_frh_combined.patch44.36 KBFrando
#1 modulesearch1.png131.8 KBFrando
#1 modulesearch2.png115.2 KBFrando
vt_modules_search_frh_combined.patch31.79 KBFrando
vt_modules_search_frh.patch9.43 KBFrando
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Frando’s picture

FileSize
115.2 KB
131.8 KB

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

Frando’s picture

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.

Status: Needs review » Needs work

The last submitted patch failed testing.

Frando’s picture

Status: Needs work » Needs review
Issue tags: +Usability
FileSize
44.36 KB
11.7 KB

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.

Frando’s picture

FileSize
113.07 KB
125.23 KB
137.92 KB

screenshots

Status: Needs review » Needs work

The last submitted patch failed testing.

moshe weitzman’s picture

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

rickvug’s picture

Issue tags: +User interface

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.

catch’s picture

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?

moshe weitzman’s picture

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.

greenSkin’s picture

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.

Frando’s picture

Status: Needs work » Needs review
FileSize
11.97 KB

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.

kkaefer’s picture

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.

Bevan’s picture

Issue tags: +Needs usability review

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.

Bevan’s picture

Issue tags: +vertical tabs

tagging

Bojhan’s picture

Status: Needs review » Needs work

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.

chx’s picture

ica’s picture

Category: bug » feature
Status: Needs work » Active

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

chx’s picture

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.

mcrittenden’s picture

Status: Active » Needs work

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

yoroy’s picture

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

Bojhan’s picture

Title: searchable modules page with vertical tabs! » Searchable modules page
Assigned: Frando » Unassigned
Priority: Normal » Critical

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.

kkaefer’s picture

Status: Needs work » Needs review
FileSize
4.31 KB

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.

kkaefer’s picture

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

RobLoach’s picture

Awesome patches in the Drupal 7 issue queue!

sun’s picture

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.

yoroy’s picture

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

kkaefer’s picture

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?

mcrittenden’s picture

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.

Dries’s picture

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.

chx’s picture

This is UX which should be fine even past slush.

mattyoung’s picture

.

Bojhan’s picture

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.

SeanBannister’s picture

sub

kkaefer’s picture

FileSize
6.14 KB

Still doesn't detect pastes correctly.

skilip’s picture

Status: Needs review » Needs work

The last submitted patch failed testing.

ksenzee’s picture

Status: Needs work » Needs review

HEAD was broken. Resetting status.

skilip’s picture

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.

Status: Needs review » Needs work

The last submitted patch failed testing.

eigentor’s picture

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

eigentor’s picture

Issue tags: +D7UX

added d7ux tag

Scott Reynolds’s picture

@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

Bojhan’s picture

Status: Needs work » Needs review

Can anyone still work on this?

eigentor’s picture

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

eigentor’s picture

FileSize
5.85 KB

Rerolled Konstantins Patch.

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

Dries’s picture

Why not use your browser's search feature?

skilip’s picture

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

eigentor’s picture

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

yched’s picture

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

eigentor’s picture

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

heather’s picture

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?

eigentor’s picture

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.

heather’s picture

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.

dawehner’s picture

FileSize
5.63 KB

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

heather’s picture

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

eigentor’s picture

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

sun’s picture

Status: Needs review » Needs work
+++ 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?

dawehner’s picture

Status: Needs work » Needs review
FileSize
5.63 KB

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

dawehner’s picture

Status: Needs review » Needs work

Reset status.

eigentor’s picture

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.

kkaefer’s picture

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

mcrittenden’s picture

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

kkaefer’s picture

FileSize
5.73 KB

Updated to resolve patch error

dawehner’s picture

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

skilip’s picture

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.

kkaefer’s picture

eigentor’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

skilip’s picture

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

eigentor’s picture

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

eigentor’s picture

Status: Needs work » Needs review
FileSize
5.99 KB

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?

greenSkin’s picture

Could name it elementFilter.

catch’s picture

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.

greenSkin’s picture

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

skilip’s picture

FileSize
8.69 KB

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.

Status: Needs review » Needs work

The last submitted patch failed testing.

skilip’s picture

Status: Needs work » Needs review
FileSize
8.72 KB

Rerolled, hopefully fixing last test failures.

Status: Needs review » Needs work

The last submitted patch failed testing.

skilip’s picture

Status: Needs work » Needs review
FileSize
8.72 KB

Bummer!!

mcrittenden’s picture

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

kkaefer’s picture

skilip’s picture

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

mcrittenden’s picture

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

Nice!

eigentor’s picture

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

would like a Review from the cody persons.

Sun? dereine?

stella’s picture

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?

skilip’s picture

@stella: which patch did you review?

stella’s picture

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.

yoroy’s picture

Status: Needs review » Needs work

thank you stella. Needs work for last touchups then.

skilip’s picture

Status: Needs work » Needs review
FileSize
8.72 KB

@stella: Thanks for reviewing!

Status: Needs review » Needs work

The last submitted patch failed testing.

skilip’s picture

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

catch’s picture

Status: Needs work » Needs review

HEAD is broken.

yched requested that failed test be re-tested.

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

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.

kkaefer’s picture

Status: Reviewed & tested by the community » Needs work
  • 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.
sun’s picture

Status: Needs work » Needs review
+++ 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?

sun’s picture

Additionally....

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

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

kkaefer’s picture

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

skilip’s picture

FileSize
6.27 KB

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.

sun’s picture

Status: Needs review » Needs work

Patch contains the JS only.

eigentor’s picture

Status: Needs work » Needs review

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.

skilip’s picture

FileSize
9.44 KB

Sorry :-[

skilip’s picture

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?

eigentor’s picture

Status: Needs review » Reviewed & tested by the community

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

eigentor’s picture

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

yoroy’s picture

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

eigentor’s picture

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

mrfelton’s picture

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.

eigentor’s picture

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

mrfelton’s picture

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

eigentor’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
9.39 KB

Updated the patch to keep up with head

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Works

eigentor’s picture

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.

chx’s picture

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

Status: Reviewed & tested by the community » Needs review

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

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Still green

aspilicious’s picture

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

eigentor’s picture

Dries have mercy on this issue :)

aspilicious’s picture

Dries as a fellow belgian citizen...
Studying at a belgian university just like you did...
I'm asking you...

commit this... XD

yoroy’s picture

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.

webchick’s picture

Status: Reviewed & tested by the community » Needs review

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.

Jon Nunan’s picture

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.
skilip’s picture

I'd prefer adding a class to each table drawer which can be ignored. Something like 'tablefilter-ignore'.

kkaefer’s picture

My original patch at http://drupal.org/node/396478#comment-2096322 had an ignore feature. Not sure why it got removed during the process.

kkaefer’s picture

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.

Dave Reid’s picture

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

casey’s picture

Assigned: Unassigned » casey

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.

skilip’s picture

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

kkaefer’s picture

Actually it's the other way around. Restricting it to table rows requires additional code.

skilip’s picture

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?

skilip’s picture

Yes, I'm stubborn. Sorry 'bout that :(.

Bojhan’s picture

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.

skilip’s picture

@Bojhan: I didn't reopen the discussion. I'm just trying to avoid more delay.

casey’s picture

FileSize
9.83 KB

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.

Status: Needs review » Needs work

The last submitted patch, instantfilter.patch, failed testing.

casey’s picture

Status: Needs work » Needs review
FileSize
9.83 KB

grmbl

skilip’s picture

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

Status: Needs review » Needs work

The last submitted patch, instantfilter_ 396478#138.patch, failed testing.

skilip’s picture

Status: Needs work » Needs review
FileSize
9.87 KB

Removed hash in filename

Status: Needs review » Needs work
Issue tags: -Needs usability review, -Usability, -User interface, -vertical tabs, -D7UX

The last submitted patch, instantfilter_ 396478_140.patch, failed testing.

Status: Needs work » Needs review
Issue tags: +Needs usability review, +Usability, +User interface, +vertical tabs, +D7UX

Re-test of instantfilter_ 396478_140.patch from comment #140 was requested by Bojhan.

casey’s picture

FileSize
10.88 KB

Minor update.

- cleaner form array
- added back name of textfield

aspilicious’s picture

Works very smooth, hopefully this baby is finished by the 15th

mcrittenden’s picture

Status: Needs review » Needs work

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.

Bojhan’s picture

Status: Needs work » Needs review

Putting back to needs review apart from those comment reviews, what about the actual code?

eigentor’s picture

The filter does not work anymore for me in Firefox 3.5 (neither #137 nor #143). Working fine in IE8. Please someone cross-check.

dmitrig01’s picture

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

dmitrig01’s picture

Status: Needs review » Needs work

The code is good. But we need comments.

casey’s picture

Status: Needs work » Needs review
FileSize
9.26 KB

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.

Status: Needs review » Needs work

The last submitted patch, instantfilter1a.patch, failed testing.

casey’s picture

Status: Needs work » Needs review
FileSize
9.25 KB
casey’s picture

Assigned: casey » Unassigned
skilip’s picture

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.

Noyz’s picture

subscribing

RobLoach’s picture

I've played around with it, and like it a lot. My initial thoughts?

  1. Are there any other places in Drupal core where we could use the Instafilter? Anything that needs a pager makes this irrelevant unfortunately. I'd love to see this on the user/content/taxonomy pages though.
  2. An autocomplete widget is part of jQuery UI 1.8, might be neat to add autocomplete to this so that you could get even more granular with it. Out of scope for this issue.
  3. Is there anything missing from the #states system that could make this be accomplished via states? We should reuse as much code as possible here.
  4. This might be a good place to push in the .js-show class. The "js-show" class only displays the element when JavaScript is available....
    .js-show {
      display: none;
    }
    html.js .js-show {
      display: inline;
    }
    
  5. This still works when on different languages, correct?
mcrittenden’s picture

Are there any other places in Drupal core where we could use the Instafilter? Anything that needs a pager makes this irrelevant unfortunately. I'd love to see this on the user/content/taxonomy pages though.

Might be nice on the permissions page.

skilip’s picture

I've already tested this on the permissions page and it works like a charm. GET THIS IN!! :P

casey’s picture

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: Allow table row groups in table.html.twig and template_preprocess_table() for that but it requires a api changes so it is pushed to D8; maybe worth an exception?

webchick’s picture

Version: 7.x-dev » 8.x-dev

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?

skilip’s picture

What a shame indeed. We had this RTBC before code freeze.

webchick’s picture

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

skilip’s picture

You're absolutely right. Don't mind me, just moaning.

casey’s picture

FileSize
20.45 KB

Added instantfilter to permissions page. Therefor I merged patch from #31535: Allow table row groups in table.html.twig and template_preprocess_table() into this one. Just so you could try it out...

sun’s picture

Please coordinate with me + http://drupal.org/project/admin_menu re: contrib.

Bojhan’s picture

@sun Honestly this doesn't belong in admin_menu.

casey’s picture

skilip’s picture

@casey: Will you mention the guys who helped here on your project page?

casey’s picture

You are absolutely right! I updated the module page.

lpalgarvio’s picture

reminding of Module filter - http://drupal.org/project/module_filter
just in case it's of any help for this patch

catch’s picture

Priority: Critical » Major

Downgrading all D8 criticals to major per http://drupal.org/node/45111

Kiphaas7’s picture

sub

xjm’s picture

Priority: Major » Normal

Status: Needs review » Needs work

The last submitted patch, instantfilter1c.patch, failed testing.

yoroy’s picture

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.

xjm’s picture

Priority: Normal » Major

Totally an accident. I just meant to retest the patch (needs a reroll). I blame gremlins. :)

klonos’s picture

...subscribing.

nicl’s picture

sub

eigentor’s picture

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?

dawehner’s picture

Status: Needs work » Needs review
FileSize
20.25 KB

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.

Status: Needs review » Needs work

The last submitted patch, instantfilter1c.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
16.74 KB

Addapting the tests, let's have a look.

Status: Needs review » Needs work

The last submitted patch, instantfilter1c_0.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
20.24 KB

Okay here is one which doesn't drop permissions

eigentor’s picture

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.

Bojhan’s picture

Yhea the consensus is only on project names.

klonos’s picture

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.

dawehner’s picture

FileSize
20.26 KB

It makes absolute sense to not filter by the module description.

Kiphaas7’s picture

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.

Bojhan’s picture

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

eigentor’s picture

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

Kiphaas7’s picture

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

Jon Nunan’s picture

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?

Bojhan’s picture

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

RobLoach’s picture

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

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.

Bojhan’s picture

@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 :)

eigentor’s picture

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.

klonos’s picture

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

klonos’s picture

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

dawehner’s picture

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.

Bojhan’s picture

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?

Kiphaas7’s picture

@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

klonos’s picture

@dereine:

A normal user for example will not care that much about getting all modules which require ctools.

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

...it looks like by filtering module name and description we should tackle the 80/90% use case.

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 can tackle the other 10%, which is more advanced.

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

... The form becomes less easy to scan ... all we are doing is causing additional problems for the 80% of our users, to satisfy a need of very few. ...

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.

klonos’s picture

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

Kiphaas7’s picture

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.

dawehner’s picture

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

Bojhan’s picture

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

Kiphaas7’s picture

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

yoroy’s picture

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

dawehner’s picture

Status: Needs review » Needs work
+++ 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.

Bojhan’s picture

Bojhan’s picture

Issue tags: -Needs usability review

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.

Bojhan’s picture

Also noticed, when you search the permission page - the table header changes (probably the lacking descriptions?) anyways that makes it look a bit wonky.

klonos’s picture

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:

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

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

...because contrib hasn't dealt with the 20% use case yet, and thus we didn't think of the newcomers? ...

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

... But especially newcomers would fall right into that 80%, so I don't really understand your argument there.

Please read my comment in #203 more thoroughly if still unclear.

Kiphaas7’s picture

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

klonos’s picture

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

webchick’s picture

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.

dawehner’s picture

Status: Needs work » Needs review
FileSize
20.97 KB

Here is a new patch with the changes mentioned in http://drupal.org/node/396478#comment-5054382
Manual testing worked as expected.

dawehner’s picture

dawehner’s picture

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.

sun’s picture

Status: Needs review » Postponed

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.

Kiphaas7’s picture

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.

casey’s picture

klonos’s picture

Issue tags: -D7UX +d8ux

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

yoroy’s picture

Would #229193: Incremental filter for permissions page be good to work on the specific JS library?

yoroy’s picture

RobLoach’s picture

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.

jenlampton’s picture

Issue tags: -vertical tabs

Updated the issue summary. And the tags.

jenlampton’s picture

Issue summary: View changes

Issue summary initiative

Bevan’s picture

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.

marvil07’s picture

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.

jenlampton’s picture

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

Taxoman’s picture

Taxoman’s picture

Issue summary: View changes

updated summary, removed typo, clarified remaining tasks

Bojhan’s picture

Per @sun's comment

1) #31535: Allow table row groups in table.html.twig and template_preprocess_table()

2) Still needs to be created

3) #396478: Searchable modules page

This issue will remain postponed till 1,2 are near completion.

Kiphaas7’s picture

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:

  1. regular search form, process server side, enhance by fetching the search results via ajax (the likes of module filter etc..)
    • Responsiveness depends on the time it takes to fetch the ajax request/reload page
    • Reloads the module page on every request. Forgot what happens behind the scenes, but it takes significantly longer to load the modules page than any other page on my setup, so I'm guessing this puts an extra burden on the server
    • Takes no effort at all to implement a non-js fallback
    • Server side filtering probably offers extra filtering possibilities. Question is if we need them though
    • Extending/altering is easy because of the drupal hook/alter system
    • Accessibility issues. Accessibility needs some thought because the table is updated dynamically.
  2. JS only filtering (the likes of instant filter etc..)
    • potential responsiveness in milliseconds. Instant filter isn't quite there yet though for searching through more than 1000 items.
    • Puts the burden on the client browser. No extra load on the server.
    • Requires extra effort to implement non-js filtering. Question remains if a non-js fallback should be offered at all.
    • Filtering options require well-formed, consistent, semantically rich tables. The js solution can obviously only base its filtering on data offered in the HTML. Some direction can be given via server side set variables which portions of HTML to include/exclude
    • Extending/altering should be offered, but AFAIK there isn't a common altering/extending system for JS functions/modules in drupal. There is a rudimentary altering system present in instant filter.
    • Mobile/accessibility issues. Mobile needs testing, and acceptable performance. Accessibility needs some thought because the table is updated dynamically.

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.

chx’s picture

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.

ksenzee’s picture

I wholeheartedly agree with Kiphaas7 and chx that JS-only is the way to go, for the same reasons they listed.

Taxoman’s picture

in favor of JS-only

klonos’s picture

Mobile/accessibility issues...

I'm not against supporting it, but honestly now... who does serious admin work on a regular basis on mobile? Especially enabling/disabling modules.

SeanBannister’s picture

@klonos Maybe not much on a mobile phone but you'll find more and more people doing it on tablet devices.

klonos’s picture

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.

Xano’s picture

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.

klonos’s picture

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

Kiphaas7’s picture

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

chx’s picture

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.

skilip’s picture

[completely-offtopic]Whoohaahaaahaa!! That's the most sensible thing I've heard about JS for UI's in ages! 1+ 4 chx[/completely-offtopic]

klonos’s picture

I really hate that this is not moving for so long now. I don't see why we don't simply implement it since it will be one less task for #538904: D8UX: Redesign Modules Page (which is also not moving and should be split to smaller issues). In the meantime, module_filter (counting a whooping 43k+ installations) has been an irreplaceable module for me in every setup (that and admin_menu).

Anyways, this was set to postponed about the same time one year ago (back in #221), reasons being that it seemed to trying to introduce 3 different things:

1) Enhancement of themed tables to contain a concept of "rowgroups"...

2) Introduction of a JS library (+ Drupal integration helpers) that's able to quickly filter a table...

3) Making the modules page searchable/filterable...

So, it was suggested to further split this issue to three smaller issues. Now we have:

1) #31535: Allow table row groups in table.html.twig and template_preprocess_table()

2) In #1475204-5: [META] Provide a generic search/filter UI interface pattern for listing pages I proposed adding Instant filter or implementing something similar and make it generic so that it can be used in other places besides the modules admin page

3) #396478: Searchable modules page (this issue here)

Bottom line is: are there any reasons for still keeping this postponed and thus of our radars?

eigentor’s picture

So why exaxtly is this issue postponed?
I know there is another one #229193: Incremental filter for permissions page , but since this one has been more active, it should be re-opened and the other one marked as duplicate.

klonos’s picture

This issue here is about implementing a search filter on the modules page while the other one you link to is about the permissions page. So not a duplicate at all.

Now, if we were to implement a global, reusable solution that would be used across multiple admin pages, then I guess we'd go with #1475204: [META] Provide a generic search/filter UI interface pattern for listing pages and #1757618: Add Instant filter functionality in D8 core..

yoroy’s picture

Views is in core. Its UI is full of searches and filters for different kinds of lists. Any chance we can apply some of that wisdom here?

eigentor’s picture

I think views dies not have that kind of real-time filter. But an initiative to implement a generic solution (and maybe convert modules page into a view?) might get some sympathy.

Having an instant filter fir views exposed filters as a display mode would have many more use cases than this one. Currently better exposed filters (s contribution module) does a full page reload if set to autosubmit.

Maybe a contribution solution could open a back door for that functionality?
Is converting modules page to a view even feasible?
Given all the hairy accessibility problems I somehow doubt it. But wouldn't it be great.

moshe weitzman’s picture

The list of enabled/disabled modules is not longer in a table. I don't think Views is useful for building this page. IMO, a clientside filter would work fine here.

Also, the Modules page just got redesigned so would be great to pick this back up.

yoroy’s picture

yeah, didn't mean to suggest to make the list of modules a view. But client side filters are used in views ui like the filter on the list of fields when adding one or more fields

Bojhan’s picture

Status: Postponed » Active

Lets reopen this

ro-no-lo’s picture

@Bojhan: You better close this one here and do a follow up, like in the module page issue. This here will hit the page size limit pretty soon and then everybody has to click one more time [next >>] at the bottom on the page to get to the newest entries.

As a sidenote:
Clicking "new" in my dashboard does not bring me to the 2nd page of a huge issue to the newest entries. Therefore I suggest a close and follow up.

yoroy’s picture

Better to agree on a certain direction first before moving to a new issue or we risk doing it all over again in the followup.

bleen’s picture

I suggest that we focus on just the search box (a la module_filter) and that's it. Lets not get into the weeds again about weather or not we should have the vertical tabs from module filter included as well as that is where most of the controversy came from in the other issue ... The one thing that was (almost) universally agreed upon there was including the search box that automatically filters ... That would be a huge win and if we do it right we can expand that paradigm to other pages like the permissions page but that's something we can talk about later....

Anyone disagree?

yoroy’s picture

What bleen18 said. Focus only on filtering the list on a given string. Loving it already :)

lpalgarvio’s picture

for permissions, i created a master issue
#1765576: Redesign Permissions Page

nod_’s picture

I'm guessing the new module page will make that issue less important ctrl+f works now. Technically, only looking at the title of this issue, it's fixed :D

As far as what to do if we want to go further, agreed with #256

Bojhan’s picture

I'd love to open a new issue, if nod_ and or bleen18 can work togheter to make a good patch.

bleen’s picture

I'll probably play with putting together a patch today

corey.aufang’s picture

I had started work on a generic JS filter using the same technique I used in FPA with the intention of having the same functionality on the module admin page, or possible any page that required client-side filtering.

The components that make up FPA are planed to be optional so you could have any combination of search box, vertical tabs, or column filter, with simple configuration changes.

I don't have the code on this machine but I can upload it later if there is any interest.

bleen’s picture

I posted a patch for this last week ... Forgot to post a follow up here to point people to it I guess

#1848064: Allow to filter modules by arbitrary search strings on the Modules page

Bojhan’s picture

Status: Active » Fixed

Whoo :D

skilip’s picture

Nice!!!

klonos’s picture

Thanx people!!! :D

Status: Fixed » Closed (fixed)

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

klonos’s picture

klonos’s picture

Updated issue summary.