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.
Comment | File | Size | Author |
---|---|---|---|
#218 | 396478-instafilter.patch | 20.97 KB | dawehner |
#188 | 396478-instafilter.patch | 20.26 KB | dawehner |
#184 | instantfilter1c.patch | 20.24 KB | dawehner |
#182 | instantfilter1c_0.patch | 16.74 KB | dawehner |
#180 | instantfilter1c.patch | 20.25 KB | dawehner |
Comments
Comment #1
Frando CreditAttribution: Frando commentedscreenshots
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".
Comment #2
Frando CreditAttribution: Frando commentedIt 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.
Comment #4
Frando CreditAttribution: Frando commentedNew 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.
Comment #5
Frando CreditAttribution: Frando commentedscreenshots
Comment #7
moshe weitzman CreditAttribution: moshe weitzman commentedOh my - lets *please* refresh this patch and push it in.
Comment #8
rickvug CreditAttribution: rickvug commentedSome 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.
Comment #9
catchJust 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?
Comment #10
moshe weitzman CreditAttribution: moshe weitzman commentedsearch 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.
Comment #11
greenSkin CreditAttribution: greenSkin commentedI'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.
Comment #12
Frando CreditAttribution: Frando commentedSo 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.
Comment #13
kkaefer CreditAttribution: kkaefer commentedI'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.
Comment #14
Bevan CreditAttribution: Bevan commentedI 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.
Comment #15
Bevan CreditAttribution: Bevan commentedtagging
Comment #16
Bojhan CreditAttribution: Bojhan commentedI 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.
Comment #17
chx CreditAttribution: chx commented#229193: Incremental filter for permissions page
Comment #18
ica CreditAttribution: ica commentedwhile 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
Comment #19
chx CreditAttribution: chx commentedI 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.
Comment #20
mcrittenden CreditAttribution: mcrittenden commented+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.
Comment #21
yoroy CreditAttribution: yoroy commentedhttp://drupal.org/node/538904 has more general modules page discussion and outlines goals we want to achieve when redesigning it.
Comment #22
Bojhan CreditAttribution: Bojhan commentedChanging 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.
Comment #23
kkaefer CreditAttribution: kkaefer commentedThis is the initial version of InstaSearch.
There are three types of elements:
instasearch-container
.instasearch-item
becomes an item.instasearch-ignore
.Doesn’t tackle the zebra striping problem.
Comment #24
kkaefer CreditAttribution: kkaefer commentedThe search field should probably be formatted in a more visually appealing way.
Comment #25
RobLoachAwesome patches in the Drupal 7 issue queue!
Comment #26
sunWe 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.
Comment #27
yoroy CreditAttribution: yoroy commentedsun: having a filter like this is part of that revamp
Comment #28
kkaefer CreditAttribution: kkaefer commentedsun: 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?
Comment #29
mcrittenden CreditAttribution: mcrittenden commentedI'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.
Comment #30
Dries CreditAttribution: Dries commentedGiven 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.
Comment #31
chx CreditAttribution: chx commentedThis is UX which should be fine even past slush.
Comment #32
mattyoung CreditAttribution: mattyoung commented.
Comment #33
Bojhan CreditAttribution: Bojhan commentedEither 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.
Comment #34
SeanBannister CreditAttribution: SeanBannister commentedsub
Comment #35
kkaefer CreditAttribution: kkaefer commentedStill doesn't detect pastes correctly.
Comment #36
skilip CreditAttribution: skilip commentedSubscribing
http://drupal.org/node/598738#comment-2132216
Comment #38
ksenzeeHEAD was broken. Resetting status.
Comment #39
skilip CreditAttribution: skilip commentedHow 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.
Comment #41
eigentor CreditAttribution: eigentor commented#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...
Comment #42
eigentor CreditAttribution: eigentor commentedadded d7ux tag
Comment #43
Scott Reynolds CreditAttribution: Scott Reynolds commenteddiff -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
Comment #44
Bojhan CreditAttribution: Bojhan commentedCan anyone still work on this?
Comment #45
eigentor CreditAttribution: eigentor commented@Bojhan we just gotta get someone to reroll one of the two patches.
Comment #46
eigentor CreditAttribution: eigentor commentedRerolled Konstantins Patch.
It appears to work fine, also when you paste somthing into the search box.
Comment #47
Dries CreditAttribution: Dries commentedWhy not use your browser's search feature?
Comment #48
skilip CreditAttribution: skilip commented@Dries: CTRL + F is just a workaround for a usability problem of the module's page.
Comment #49
eigentor CreditAttribution: eigentor commented@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.
Comment #50
yched CreditAttribution: yched commentedAdditionally, CTRL-F forces you to tab through false positives that appear "somewhere" in the page - notably hidden in admin_menu's menu tree.
Comment #51
eigentor CreditAttribution: eigentor commentedyes, the filtering down is nice and clean - leaves only few results on the page.
Comment #52
heather CreditAttribution: heather commentedThis 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?
Comment #53
eigentor CreditAttribution: eigentor commentedI 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.
Comment #54
heather CreditAttribution: heather commentedEigentor 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.
Comment #55
dawehnerI 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
Comment #56
heather CreditAttribution: heather commented@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.
Comment #57
eigentor CreditAttribution: eigentor commented#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...
Comment #58
sunThis 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?
Comment #59
dawehnerUpdate:
- 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
Comment #60
dawehnerReset status.
Comment #61
eigentor CreditAttribution: eigentor commentedThere 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.
Comment #62
kkaefer CreditAttribution: kkaefer commented"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!
Comment #63
mcrittenden CreditAttribution: mcrittenden commented@dereine, did you attach the right patch in #59? Looks like the changes you mentioned weren't actually changed.
Comment #64
kkaefer CreditAttribution: kkaefer commentedUpdated to resolve patch error
Comment #65
dawehnerkkaefer: you missed to add --no-prefix to git diff
Comment #66
skilip CreditAttribution: skilip commentedSorry 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.
Comment #67
kkaefer CreditAttribution: kkaefer commented@skilip: #229193: Incremental filter for permissions page
Comment #68
eigentor CreditAttribution: eigentor commentedComment #70
skilip CreditAttribution: skilip commented@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.
Comment #71
eigentor CreditAttribution: eigentor commentedskilip: 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).
Comment #72
eigentor CreditAttribution: eigentor commentedUpdated 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?
Comment #73
greenSkin CreditAttribution: greenSkin commentedCould name it elementFilter.
Comment #74
catchJust 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.
Comment #75
greenSkin CreditAttribution: greenSkin commented@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.
Comment #76
skilip CreditAttribution: skilip commentedHopefully 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.
Comment #78
skilip CreditAttribution: skilip commentedRerolled, hopefully fixing last test failures.
Comment #80
skilip CreditAttribution: skilip commentedBummer!!
Comment #81
mcrittenden CreditAttribution: mcrittenden commentedGood 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).
Comment #82
kkaefer CreditAttribution: kkaefer commented@mcrittenden: #229193: Incremental filter for permissions page
Comment #83
skilip CreditAttribution: skilip commented@mcrittenden: the filter can be applied to all tables, or groupings of tables.
Comment #84
mcrittenden CreditAttribution: mcrittenden commentedNice!
Comment #85
eigentor CreditAttribution: eigentor commentedCan just review the function: works just as well as Konstantins patch.
would like a Review from the cody persons.
Sun? dereine?
Comment #86
stella CreditAttribution: stella commentedThis 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?
Comment #87
skilip CreditAttribution: skilip commented@stella: which patch did you review?
Comment #88
stella CreditAttribution: stella commentedCrap, 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.
Comment #89
yoroy CreditAttribution: yoroy commentedthank you stella. Needs work for last touchups then.
Comment #90
skilip CreditAttribution: skilip commented@stella: Thanks for reviewing!
Comment #92
skilip CreditAttribution: skilip commentedCan anyone explain to me why this patch failed on 'ForumTestCase'?
Comment #93
catchHEAD is broken.
Comment #95
Bojhan CreditAttribution: Bojhan commentedI 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.
Comment #96
kkaefer CreditAttribution: kkaefer commentedcontainer[0].tagName.toLowerCase() == 'form'
Please use something like
container.is('form')
instead of relying on the DOM....attachEventListeners = function() {
Do we really need a separate function for two lines of code that is only called in one location?
Drupal.tablefilters[settings.id] = new Drupal.tableFilter(container, settings);
Drupal.tablefilters[settings.id].init();
Why do we have to call init separately instead of calling it in the constructor?
prototype
but extend the object withthis.ref
?Drupal.tableDrag.prototype.restripeTable
). We should refactor that code into a more generic function and reuse it instead of duplicating it.Comment #97
sunuh 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?
Comment #98
sunAdditionally....
Did anyone consider http://rikrikrik.com/jquery/quicksearch ?
Compared to the code here, that looks quite advanced + awesome.
Comment #99
kkaefer CreditAttribution: kkaefer commentedNot 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).
Comment #100
skilip CreditAttribution: skilip commentedOk, 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.
Comment #101
sunPatch contains the JS only.
Comment #102
eigentor CreditAttribution: eigentor commentedEr - 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.
Comment #103
skilip CreditAttribution: skilip commentedSorry :-[
Comment #104
skilip CreditAttribution: skilip commentedI'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?
Comment #105
eigentor CreditAttribution: eigentor commentedWhile 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...
Comment #106
eigentor CreditAttribution: eigentor commentedhm this has been sitting in RTBC for quite a while... Will it make it in?
Comment #107
yoroy CreditAttribution: yoroy commentedPatience my dear! Core committers have a snapshot of what was RTBC on 1 december, they will get to it eventually.
Comment #108
eigentor CreditAttribution: eigentor commentedyoroy ah :)
I thought of something like that.
Hehe Zielfoto
Comment #109
mrfelton CreditAttribution: mrfelton commented1 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.
Comment #110
eigentor CreditAttribution: eigentor commentedmrfelton 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 ;)
Comment #111
mrfelton CreditAttribution: mrfelton commented@eigentor: I'm 100% sure this is a good feature! Just needs a little tuning in my eyes.
Comment #112
eigentor CreditAttribution: eigentor commentedUpdated the patch to keep up with head
Comment #113
Bojhan CreditAttribution: Bojhan commentedWorks
Comment #114
eigentor CreditAttribution: eigentor commentedAs 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.
Comment #115
chx CreditAttribution: chx commentedIf you think this is unnecessary try to find views which half of the world depends on...
Comment #117
Bojhan CreditAttribution: Bojhan commentedStill green
Comment #118
aspilicious CreditAttribution: aspilicious commentedMy first patch applied succesfully i can confirm this is working great. Love this thing.
Comment #119
eigentor CreditAttribution: eigentor commentedDries have mercy on this issue :)
Comment #120
aspilicious CreditAttribution: aspilicious commentedDries as a fellow belgian citizen...
Studying at a belgian university just like you did...
I'm asking you...
commit this... XD
Comment #121
yoroy CreditAttribution: yoroy commentedeigentor'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.
Comment #122
webchickI 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.
Comment #123
Jon Nunan CreditAttribution: Jon Nunan commentedCan always use the jQuery .not() to ignore chunks of text. For example changing
to
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?
Comment #124
skilip CreditAttribution: skilip commentedI'd prefer adding a class to each table drawer which can be ignored. Something like 'tablefilter-ignore'.
Comment #125
kkaefer CreditAttribution: kkaefer commentedMy original patch at http://drupal.org/node/396478#comment-2096322 had an ignore feature. Not sure why it got removed during the process.
Comment #126
kkaefer CreditAttribution: kkaefer commentedOh, 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.
Comment #127
Dave ReidI 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!"
Comment #128
casey CreditAttribution: casey commentedOk 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.
Comment #129
skilip CreditAttribution: skilip commented@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.
Comment #130
kkaefer CreditAttribution: kkaefer commentedActually it's the other way around. Restricting it to table rows requires additional code.
Comment #131
skilip CreditAttribution: skilip commentedIs 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?
Comment #132
skilip CreditAttribution: skilip commentedYes, I'm stubborn. Sorry 'bout that :(.
Comment #133
Bojhan CreditAttribution: Bojhan commentedskilip: 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.
Comment #134
skilip CreditAttribution: skilip commented@Bojhan: I didn't reopen the discussion. I'm just trying to avoid more delay.
Comment #135
casey CreditAttribution: casey commentedOk initial patch. Took longer than I hoped (mostly because of the zebra updating).
I'll continue tomorrow, but I appreciate reviews. Patch is usable.
Comment #137
casey CreditAttribution: casey commentedgrmbl
Comment #138
skilip CreditAttribution: skilip commentedI'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!!
Comment #140
skilip CreditAttribution: skilip commentedRemoved hash in filename
Comment #143
casey CreditAttribution: casey commentedMinor update.
- cleaner form array
- added back name of textfield
Comment #144
aspilicious CreditAttribution: aspilicious commentedWorks very smooth, hopefully this baby is finished by the 15th
Comment #145
mcrittenden CreditAttribution: mcrittenden commentedNot sure if this is still a work in progress but I gave it a little "comment review" nonetheless.
Typo here.
Be consistent with grammar in comments (Remove vs. Removes, etc.)
Typo here.
Needs a period.
Obviously waiting on comments.
Comment #146
Bojhan CreditAttribution: Bojhan commentedPutting back to needs review apart from those comment reviews, what about the actual code?
Comment #147
eigentor CreditAttribution: eigentor commentedThe filter does not work anymore for me in Firefox 3.5 (neither #137 nor #143). Working fine in IE8. Please someone cross-check.
Comment #148
dmitrig01 CreditAttribution: dmitrig01 commenteduse .once()
Looks like an instantFilter, not tableFilter
How is this different from $().text() ?
Other than that (and the comments above), the screencast looks good and the code looks good too.
Comment #149
dmitrig01 CreditAttribution: dmitrig01 commentedThe code is good. But we need comments.
Comment #150
casey CreditAttribution: casey commentedI 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.
Comment #152
casey CreditAttribution: casey commentedComment #153
casey CreditAttribution: casey commentedComment #154
skilip CreditAttribution: skilip commentedI 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.
Comment #155
Noyz CreditAttribution: Noyz commentedsubscribing
Comment #156
RobLoachI've played around with it, and like it a lot. My initial thoughts?
Comment #157
mcrittenden CreditAttribution: mcrittenden commentedMight be nice on the permissions page.
Comment #158
skilip CreditAttribution: skilip commentedI've already tested this on the permissions page and it works like a charm. GET THIS IN!! :P
Comment #159
casey CreditAttribution: casey commentedpermissions 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?
Comment #160
webchickSorry, but it feels absolutely way too late for this now. :( What a shame...
Can someone pull this functionality into a contrib module for D7?
Comment #161
skilip CreditAttribution: skilip commentedWhat a shame indeed. We had this RTBC before code freeze.
Comment #162
webchickWell. 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. :(
Comment #163
skilip CreditAttribution: skilip commentedYou're absolutely right. Don't mind me, just moaning.
Comment #164
casey CreditAttribution: casey commentedAdded 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...
Comment #165
sunPlease coordinate with me + http://drupal.org/project/admin_menu re: contrib.
Comment #166
Bojhan CreditAttribution: Bojhan commented@sun Honestly this doesn't belong in admin_menu.
Comment #167
casey CreditAttribution: casey commentedI created a project: http://drupal.org/project/instantfilter
Comment #168
skilip CreditAttribution: skilip commented@casey: Will you mention the guys who helped here on your project page?
Comment #169
casey CreditAttribution: casey commentedYou are absolutely right! I updated the module page.
Comment #170
lpalgarvio CreditAttribution: lpalgarvio commentedreminding of Module filter - http://drupal.org/project/module_filter
just in case it's of any help for this patch
Comment #171
catchDowngrading all D8 criticals to major per http://drupal.org/node/45111
Comment #172
Kiphaas7 CreditAttribution: Kiphaas7 commentedsub
Comment #173
xjmComment #175
yoroy CreditAttribution: yoroy commentedPlease 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.
Comment #176
xjmTotally an accident. I just meant to retest the patch (needs a reroll). I blame gremlins. :)
Comment #177
klonos...subscribing.
Comment #178
nicl CreditAttribution: nicl commentedsub
Comment #179
eigentor CreditAttribution: eigentor commentedHm 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?
Comment #180
dawehnerThe 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.
Comment #182
dawehnerAddapting the tests, let's have a look.
Comment #184
dawehnerOkay here is one which doesn't drop permissions
Comment #185
eigentor CreditAttribution: eigentor commentedHave 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.
Comment #186
Bojhan CreditAttribution: Bojhan commentedYhea the consensus is only on project names.
Comment #187
klonosThere 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.
Comment #188
dawehnerIt makes absolute sense to not filter by the module description.
Comment #189
Kiphaas7 CreditAttribution: Kiphaas7 commentedThe 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.
Comment #190
Bojhan CreditAttribution: Bojhan commented@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.
Comment #191
eigentor CreditAttribution: eigentor commented@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...
Comment #192
Kiphaas7 CreditAttribution: Kiphaas7 commented@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!
Comment #193
Jon Nunan CreditAttribution: Jon Nunan commentedI 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?
Comment #194
Bojhan CreditAttribution: Bojhan commented@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).
Comment #195
RobLoachIf 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.
Comment #196
Bojhan CreditAttribution: Bojhan commented@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 :)
Comment #197
eigentor CreditAttribution: eigentor commentedI 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.
Comment #198
klonos"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.
Comment #199
klonos...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.
Comment #200
dawehnerMy 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.
Comment #201
Bojhan CreditAttribution: Bojhan commentedI 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?
Comment #202
Kiphaas7 CreditAttribution: Kiphaas7 commented@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
Comment #203
klonos@dereine:
Yes they would Daniel. If they actually want to disable it but they need to disable all modules that require it first. Do you really believe this is a far-fetched case? I don't. Unless of course we manage to have in core some way to batch-disable modules with dependencies that might have more dependencies in turn and so on... (the way we do when we batch-enable module dependencies).
@Bojhan: I understand that your job title is Usability Expert so this is what you do, but...
Where did you actually get these 80%-20% or 90%-10% stats. Is this official or just a personal guess/estimation. Anyways...
"Contrib modules that already exist"??? Care to point to one that actually does this? I hope I make myself clear here: I am not speaking for myself. I can manage pretty fine the way the modules page is right now (without the filter), but I've been a drupal user for more than 2 years now. How about newcomers? Do we simply want to say "Oh, well, what can you do? Someone will eventually come with a contrib module that does what you want. Someday..."* or "Ah yes! Here's this contrib module that does what you are after."
*(basically this is what I feel we generally do to newbies in d.o. ...despite the fact that most of their requests are valid usability problems)
ok this seems fair enough, but ...still not convinced (lack of official usability study/stats and all). Even if so, I'd really love to see some actual, existing contrib project that covers the needs of this "10% of advanced audience" so we can point people to it and actually provide them with a solution.
Till this is *actually* done in contrib, my (not-so) humble opinion is that this needs to be addressed in core. I mean, unless of course there is *actual* proof that it either noticeably decreases performance or hinders UX. OTOH I'm afraid this is going to be a "core needs to be kept simple" thing so I believe I'm crying to a wall here.
Comment #204
klonos...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:
Comment #205
Kiphaas7 CreditAttribution: Kiphaas7 commentedIt 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.
Comment #206
dawehnerAs the code is OOP it should be somehow possible to extend the behaviour in a contrib module, but i'm not 100% sure.
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.".
Comment #207
Bojhan CreditAttribution: Bojhan commented@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.
Comment #208
Kiphaas7 CreditAttribution: Kiphaas7 commented#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 ;).
Comment #209
yoroy CreditAttribution: yoroy commented@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.
Comment #210
dawehnerThis identation looks kind of weird.
It would be kind of cool if there would be some kind of documentation what this does.
Not sure whether it's worth to change now, but i think drupal uses this.$foo if it's a jQuery object.
Isn't it possible to use "this" here?
Some notes about the recursion would be cool as well.
Let's use a jQuery.each with another function on the object itself + jQuery.proxy
This should be another function on the object from my perspective.
Same as above
Wouldn't something like this.currentSearch explains a bit better what it does?
This line should be documentation. For some people it's probably not clear that indexOf actually searches whether the string matches.
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?
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.
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.
Comment #211
Bojhan CreditAttribution: Bojhan commented@Dries http://www.theatlantic.com/technology/archive/2011/08/crazy-90-percent-o...
Comment #212
Bojhan CreditAttribution: Bojhan commentedThere 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.
Comment #213
Bojhan CreditAttribution: Bojhan commentedAlso noticed, when you search the permission page - the table header changes (probably the lacking descriptions?) anyways that makes it look a bit wonky.
Comment #214
klonosSorry for the late reply, I was away.
Anyways, as I said previously, I am not talking about my needs here, since I've been using Drupal for more than 2 years now. In this time I've read my share of literature and I know how to handle things + I know how to use ctrl+f ;) I am more playing the role of the devil's advocate here, trying to think as a newbie. When I ask my lady or my customers to do stuff in the Drupal UI, they seem puzzled quite often and when I explain to them how things work, I get a lot of "Why not simply have it this/that way?" feedback. So, what I do is transform most of this feedback to filed issues in the queue in d.o.
Basically, what follows is more like a generic rant - still related to this issue, but doesn't actually help solving it. So, feel free to read only if you spare the time...
@yoroy, #209:
Actually, you are wrong Roy, so I'm correcting: I didn't say "Don't fix this issue *at all*" (and thus "Don't fix -the supposedly- 80% of use cases"). I was more like "Why not also include *optional* features that will also satisfy the rest of the -again, supposedly- 20%?".
Yep you are right there, but this observation (or more like a general d.o feeling based on personal experience) wasn't focused on this issue alone. As I said back in #203, I feel that in most such cases, we simply drive newbies away by saying: "Oh, well, what can you do? Someone will eventually come with a contrib module that does what you want. Someday...", while we should be more like: "Ah yes! Here's this contrib module (or workaround) that does what you are after.".
...point being that we should be providing people with actual solutions/workarounds (when we can) instead of asking them to hope & pray for a solution in the future. What is most disturbing with this behavior of ours is not my feeling that some people (mainly project maintainers) have this tendency to quickly close issues, rather than the fact that some of them also have the confidence that they actually helped the person asking for help, when -for example- they simply mark the issue as "fixed" instead of "postponed" or "closed (won't fix)". They need to understand that "fixed" means precisely that!! - not "will soon/someday be fixed ...by someone" or "closed because I don't like open issues in my queue".
Please read my comment in #203 more thoroughly if still unclear.
Comment #215
Kiphaas7 CreditAttribution: Kiphaas7 commented#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.
Comment #216
klonos...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."
Comment #217
webchickI'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.
Comment #218
dawehnerHere is a new patch with the changes mentioned in http://drupal.org/node/396478#comment-5054382
Manual testing worked as expected.
Comment #219
dawehnerhttp://drupal.org/node/396478#comment-5074824 is required
Comment #220
dawehnerIt'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.
Comment #221
sunSorry, 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.
Comment #222
Kiphaas7 CreditAttribution: Kiphaas7 commentedsun:
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.
Comment #223
casey CreditAttribution: casey commented#31535: Allow table row groups in table.html.twig and template_preprocess_table()
Comment #224
klonosIMHO, 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...
Comment #225
yoroy CreditAttribution: yoroy commentedWould #229193: Incremental filter for permissions page be good to work on the specific JS library?
Comment #226
yoroy CreditAttribution: yoroy commentedFor 1. in #221: #31535: Allow table row groups in table.html.twig and template_preprocess_table()
Comment #227
RobLoachNote 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.
Comment #228
jenlamptonUpdated the issue summary. And the tags.
Comment #228.0
jenlamptonIssue summary initiative
Comment #229
Bevan CreditAttribution: Bevan commentedI 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.
Comment #230
marvil07 CreditAttribution: marvil07 commentedA related subset issue that uses a php based filter(instead of js) and only use project name: #602528: Filter module page on project name.
Comment #231
jenlamptonI 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
Comment #232
Taxoman CreditAttribution: Taxoman commentedFYI: Related feature request: #1357662: Collaborative module categorisation and search relevance improvements
Comment #232.0
Taxoman CreditAttribution: Taxoman commentedupdated summary, removed typo, clarified remaining tasks
Comment #233
Bojhan CreditAttribution: Bojhan commentedPer @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.
Comment #234
Kiphaas7 CreditAttribution: Kiphaas7 commentedAfter talking a bit with Bojhan on IRC, we came to the following stalemate, which needs some input from javascript gurus in order to help move this issue forward:
In my mind, there are 2 fundamentally different approaches to creating a searchable modules page:
Personally, I'd lean towards the js-only solution. Not having a non-js fallback is not a problem in my mind (you just get the old situation). It does provide more challenges than the ajax solution, but if done right we also get a nice js search framework as an added bonus. Plus the responsiveness is, again if done properly, much better than the ajax solution.
EDIT: Though I don't have much time these days to work on it, I'd love to be involved in some way in moving this forward.
EDIT2: added accessibility issues to ajax solution as well.
Comment #235
chx CreditAttribution: chx commentedif 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.
Comment #236
ksenzeeI wholeheartedly agree with Kiphaas7 and chx that JS-only is the way to go, for the same reasons they listed.
Comment #237
Taxoman CreditAttribution: Taxoman commentedin favor of JS-only
Comment #238
klonosI'm not against supporting it, but honestly now... who does serious admin work on a regular basis on mobile? Especially enabling/disabling modules.
Comment #239
SeanBannister CreditAttribution: SeanBannister commented@klonos Maybe not much on a mobile phone but you'll find more and more people doing it on tablet devices.
Comment #240
klonosI 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.
Comment #241
XanoI 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.
Comment #242
klonos...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.
Comment #243
Kiphaas7 CreditAttribution: Kiphaas7 commentedIn 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 :).
Comment #244
chx CreditAttribution: chx commentedYour 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.
Comment #245
skilip CreditAttribution: skilip commented[completely-offtopic]Whoohaahaaahaa!! That's the most sensible thing I've heard about JS for UI's in ages! 1+ 4 chx[/completely-offtopic]
Comment #246
klonosI 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:
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?
Comment #247
eigentor CreditAttribution: eigentor commentedSo 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.
Comment #248
klonosThis 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..
Comment #249
yoroy CreditAttribution: yoroy commentedViews 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?
Comment #250
eigentor CreditAttribution: eigentor commentedI 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.
Comment #251
moshe weitzman CreditAttribution: moshe weitzman commentedThe 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.
Comment #252
yoroy CreditAttribution: yoroy commentedyeah, 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
Comment #253
Bojhan CreditAttribution: Bojhan commentedLets reopen this
Comment #254
ro-no-lo CreditAttribution: ro-no-lo commented@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.
Comment #255
yoroy CreditAttribution: yoroy commentedBetter to agree on a certain direction first before moving to a new issue or we risk doing it all over again in the followup.
Comment #256
bleen CreditAttribution: bleen commentedI 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?
Comment #257
yoroy CreditAttribution: yoroy commentedWhat bleen18 said. Focus only on filtering the list on a given string. Loving it already :)
Comment #258
lpalgarvio CreditAttribution: lpalgarvio commentedfor permissions, i created a master issue
#1765576: Redesign Permissions Page
Comment #259
nod_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
Comment #260
Bojhan CreditAttribution: Bojhan commentedI'd love to open a new issue, if nod_ and or bleen18 can work togheter to make a good patch.
Comment #261
bleen CreditAttribution: bleen commentedI'll probably play with putting together a patch today
Comment #262
corey.aufang CreditAttribution: corey.aufang commentedI 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.
Comment #263
bleen CreditAttribution: bleen commentedI 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
Comment #264
Bojhan CreditAttribution: Bojhan commentedWhoo :D
Comment #265
skilip CreditAttribution: skilip commentedNice!!!
Comment #266
klonosThanx people!!! :D
Comment #268
klonos...minor bug: #2054211: Modules page, Search filter: Fieldsets with no results should be hidden.
Comment #268.0
klonosUpdated issue summary.