Module looks awesome and useful, it just needs to rely on D7 core css and code more.

Just remove custom css, vertical tabs are already in core and there is no need to make custom vertical tabs. Current ones look cramped.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greenSkin’s picture

Title: UI should go with a flow » UI should utilize Drupal 7 core more
Category: feature » task

The Drupal 7 branch for Module Filter is a simple port of the Drupal 6 version which does not make dependencies on other modules. Since we now have a stable Drupal 7 port (thanks to work by webchick & dtarc) we can move to a better Drupal 7 implementation.

mansspams’s picture

cool. do want.

LarsKramer’s picture

Kiphaas7’s picture

Status: Needs review » Active
FileSize
6.17 KB

Before I knew of this module, I made a a simple vertical tabs module for the modules page. In time, I learned of instantfilter, and integrated that in my module.

Recently I had some extra time and improved my module a bit so it would mimick module_filter. It's probably not fully ready, and the javascript could use a sweep to improve readability. It also might eat kittens, etc...

It does not depend on instantfilter, but obviously if you want a search field, you need to install instantfilter beta2 or dev.

Please, try it and let me know if this is something worth pursuing in this project!

Kiphaas7’s picture

Status: Active » Needs review
Kiphaas7’s picture

Status: Active » Needs review

I just stumbled across this post:
http://drupal.org/node/1241662#comment-4836090

Using the core vertical tabs is something I would like to consider at least as an optional setting. The problem with using it rather than the current tabs implementation is it would require to either filter within the active tab OR filter based on all. The current tabs implementation lets you filter on modules within the active tab, and provides a tab that contains all modules. I was originally developing this module (the 6.x branch) 'round the time that vertical tabs for Drupal 6 was hitting its stride, and I felt it was easiest to provides tabs that when clicked would simply show/hide the necessary rows rather than hack a workable method in order to use Vertical Tabs.

greenSkin

To explain how my code handles it: instantfilter uses the search on all modules, but vertical tabs only shows the matches in the active tab. However, if it does not find any matches anymore in the current tab, it automatically switches to a tab with the most matches. Also, while searching a count displayed as searchmatches/total #modules is visible in each tab.

For me, it works pretty smooth, but then again, I'm biased. :)

Kiphaas7’s picture

FileSize
6.88 KB

Minor improvements, most notably some css improvements and collapsing the operations links in a single cell. (extra width, yay!)

klonos’s picture

Status: Needs review » Needs work

Thank you!!! It feels really nice to see my dream of help, permissions and config links merged in a single cell being realized!

A few things:

1. The margin-left: 255px; in module_filter.css for div.vertical-tabs .vertical-tabs-panes messed it up - at least in my custom theme so I had to remove it. Are you sure it is required?

2. I saw no autocomplete instant filter and at first I thought I was doing something wrong. I then recalled what this is all about (using core more) and I installed Instant Filer. Now I could see magic!! Since Instant Filer is not a requirement for this module in order to function, we cannot list it as a requirement. It would be nice though if it was mentioned in a README.txt along with the benefits it offers.

3. While figuring out 2 above, I came across this issue: selecting a checkbox (either enabling or disabling) and hitting the "enter" in my keyboard used to work and submit the form both in vanilla core and with the official version of module_filter. Now it doesn't and one has to scroll to the bottom of the page in order to locate and hit the "Save config" button.

4. The "Enabled" th in the table header makes the checkboxes column way wider than required for a single checkbox and it is such a waste of horizontal space. I wish we could rename it to something shorter like "On/Off" or "Status" or something even shorter (since we are "hacking" core with merging cells and all I mean). Ideas??

klonos’s picture

...forgot to say:

5. The "Core" tab doesn't filter modules displayed at all and behaves like the "All" tab of the official module version once a search through instant filter is initiated. There is no dedicated "All" tab at all in your version of the module.

6. I saw no count indicators for results found in each tab. Am I missing something here? Perhaps some other module I need to have enabled that you forgot to mention?

7. Results did show up but there was no filtering based on which tab I was at. I wouldn't consider this a bug at all if it auto-switched to the appropriate tab (you say back in #6 that it is smart enough to switch to the tab with the most results too!!), but I didn't actually see this feature. Again, am I missing something or doing something wrong?

klonos’s picture

...for issue 3 above in post #8: pressing the "enter" key repeatedly revealed that it seems to circle through selecting/deselecting the currently selected tab (its label).

And there's another issue:

8. The "Enabled", "Disabled", "Required" and "Unavailable" checkboxes seem to not have any effect at all in the filtered results.

klonos’s picture

James has already stated back in #2 that the 7.x-1.x was meant to be a straight port to 7.x without introducing any dependencies. This would be so much easier if there was a 7.x-2.x branch and an accompanying dev version.

klonos’s picture

9. the newly created list is rendered within a div with class="item-list" can we instead move this class to the ul and remove the div. If so, then we also need to change in module_filter.css:

/* Collapsed permissions, configure and help. */
td .item-list ul,
td .item-list ul li {
  list-style-type: none;
  margin: 0;
}

to:

/* Collapsed permissions, configure and help. */
td ul.item-list,
td ul.item-list li {
  list-style-type: none;
  margin: 0;
}
Kiphaas7’s picture

Lots of questions :).

First of all, you seem to have trouble with some of the features. Please let me know which browser you are using (version + OS) so I can actually debug this. Also, try switching to the Seven theme and let me know if the quirks you encountered still happen. (I developed it entirely in the seven theme, did not try any other themes yet, so there might be bugs related to that.)

  1. Removed it. It wasn't necessary, just to give the table some extra width. No biggie, the gain was small.
  2. Obviously it would need to be mentioned somewhere, but this is a WIP.
  3. Browser behavior is different when selecting a checkbox and hitting enter. For instance, in firefox, safari and chrome it submits the form for me, while in opera it toggles the checkbox. Only in IE (8) it does not do anything. However, going back to the instantfilter search field and hitting enter does work in all browsers I tested.
  4. If you could come up with something short, great! However, on/off or status are not good replacements. Checkboxes need a boolean question (answer should be yes(enabled) or no(disabled), exactly the two states a checkbox can have). 'Enabled' indicates just that. Status could have many more answers than enabled or disabled, while on/off isn't a question at all. 'On' could be a suitable replacement, but I don't know if I'd like that...
  5. True, there is no 'All' tab. However the 'Core' tab should not display all results, only core modules. No all tab is needed imho because it shows global search matches in the tab. Again, list your browser and switch to the seven theme and try again.
  6. No other module needed. The search indicator only shows in a tab (next to the tab name) when there is a search ongoing in the instantfilter field. So far, it did not show search results when toggling checkboxes in combination with an empty search string. I just added it, now it does.
  7. True, it only switches to a different tab if your current tab does not have matches anymore. If it would constantly switch to the tab with the most matches, the user would be going crazy...
  8. They do for me, in IE8, FF6, Chrome13, Safari5.1 and Opera 11.50. All on windows. Again, try switching themes and report back.
  9. Not really my choice either, but a consequence of using theme('item_list', $links). It is by far the easiest way in drupal to generate a list of items.
Kiphaas7’s picture

FileSize
6.93 KB

I quickly tried bartik, garland and stark, and all of theme seemed to work just fine (judging from clicking around for 1 minute).

Attached version has the changes mentioned in items 1 and 6 in post #13.

Kiphaas7’s picture

This just occured to me: you'll need instantfilter beta2 or higher for my code to work with it. Using an older version could very well explain the weird things happening to you.

Kiphaas7’s picture

Status: Needs work » Needs review
klonos’s picture

...was about to reply to all the issues, but most of them require testing on my side, so let me answer this really quickly first:

I'm on firefox 8.0a1pre x64 nightlies on Win7 x64. Will check other browsers too and report back.

As for the Instant filter version, I'm using latest dev (2011-Aug-15), but that might have been before beta2. I guess I'll either try beta2 or just give it some time till there is a fresh dev built (after the latest commit I mean) and I'll try again. Give me half a day or so...

klonos’s picture

Title: UI should utilize Drupal 7 core more » UI should utilize Drupal 7 core more - new 7.x-2.x branch?

Give me half a day or so...

...or so ;)

I'm using latest dev (2011-Aug-15), but that might have been before beta2...

The dev was/is before beta2. I was simply expecting a new dev 12 hours after committing #1251448: Capitalize first letters in Instant Filter project's name. or #1252576: Update .info file (Give it an appropriate package, remove files[]). Still no new dev though so I decided to not wait any longer and I tried installing beta2. It solved the issues I was having ...well, most of them + I still see #1251460: Doesn't work in the admin/config page - make Instant filter support two sibling items as a single item for instantfilter though.

Please consider opening a new 2.x branch and release #14 in it so I can file separate issues for the remaining problems. It is so hard to follow up here + it is just wrong to report problems in bulk in a single issue in general.

Kiphaas7’s picture

Even though I already worked some more on #14's code, this issue should be about whether or not the code in #14 is acceptable for module filter. If so, then there can be talk about when it should be pushed in a 7.x-2.x branch. If it comes that far, I'll post updated code again. No need to further spam this issue.

So far, only you (Klonos) and (obviously) me are in favor of making this a new version release of module_filter, but we both aren't in charge of that :). Let the module maintainers decide on that!

klonos’s picture

How about hiding tabs without results altogether instead of displaying 0/x? ...or at least make it an option.

If results are found in the first tab ("Core") and the very last (usually "Views"), one has to scroll quite a bit to get to the last tab with results. Personally, I see no reason for the empty tabs to show anyways.

greenSkin’s picture

I like the thought of condensing the operations into an item list. Perhaps we can move that aspect into it's own issue.

I'm not against incorporating Vertical Tabs, but I am hesitant in making it the only method of theming the modules list. Mainly because I know several people who rarely actually use the tabs when trying to find a particular module. The original intent of this module was to provide a quick and simple method for finding a module. Entering text to filter on then trying to determine the tab the module you need is in, I don't believe is the best implementation to use. Perhaps when text is entered the list should return a global result rather than only modules within the current tab. We could also provide an option that allows the user to limit the filter to the current tab, a toggle. I do like how the Vertical Tabs safely degrades back to fieldsets when JavaScript is disabled, unlike my current tabs implementation. I'm also concerned about the height of each tab. I like it except that when there are lots of tabs it requires lots of scrolling.

In the current tabs implementation, when a tab is selected a hash tag is set in the URL. This allows any particular tab to be opened on page load, and potentially provides a forward/backward history (currently not working). Potentially the form can redirect to the the URL corresponding to the active tab when the form was submitted.

I'm hesitant to rely on a separate module in order to filter. I'm more willing to make the module dependent on it than have it optional. I'd be afraid of confusion when installing "Module Filter" and there is no "filter".

My final thought for now, what is the performance difference? Simply comparing JavaScript code, Kiphaas7's is a fair bit longer (not including lines of code from Instant Filter). Without actually performing tests, I can only assume the client takes a bigger hit on performance with the proposed changes. Module Filter is often used in dev environments where it's not uncommon to have hundreds of modules available, putting stress on any filter's performance.

klonos’s picture

The original intent of this module was to provide a quick and simple method for finding a module. Entering text to filter on then trying to determine the tab the module you need is in, I don't believe is the best implementation to use...

Fair enough, James. But I really like it and I'm sure others might/will like it too. Perhaps it deserves a sub-module(?).

I'm hesitant to rely on a separate module in order to filter. I'm more willing to make the module dependent on it than have it optional. I'd be afraid of confusion when installing "Module Filter" and there is no "filter".

Again fair. Either that, or perhaps this effort here should be realized as a submodule of the Instant filter project instead. Instant filter does intent to add itself to the modules, users, permissions & config pages. These will most likely require separate settings anyways - since they filter different things and all.

...OTOH, everybody hates code/project duplication. I don't know :/

klonos’s picture

@Kiphaas7, #19: Sorry, your comment got off my radar there...

...No need to further spam this issue... ...Let the module maintainers decide on that!...

After so many issues and so many back-n-forths between the two modules' issue queues I was somehow under the impression that you co-maintained this one as well. Got it now. Sorry for the noise and the mistargeted bugging ;)

Kiphaas7’s picture

I like the thought of condensing the operations into an item list. Perhaps we can move that aspect into it's own issue.

It's a single hook_preprocess_HOOK of about 40 lines, and a fews lines of css to get it right. So yes, that could be handled in a separate issue. Glad you like it!

I'm not against incorporating Vertical Tabs, but I am hesitant in making it the only method of theming the modules list. Mainly because I know several people who rarely actually use the tabs when trying to find a particular module. The original intent of this module was to provide a quick and simple method for finding a module. Entering text to filter on then trying to determine the tab the module you need is in, I don't believe is the best implementation to use. Perhaps when text is entered the list should return a global result rather than only modules within the current tab.

Instant filter always searches 'globally', in all the tabs/fieldsets. It's my code that enforces a single fieldset to be visible. But once your search query has no more matches in the current tab, it automatically switches to the tab which has the most results for the current search. See attached image to explain that workflow without you needing to try it.

Also, search matches are shown for each tab in the place where you show how many modules got disabled/enabled.

On top of that, the autocomplete also uses global matches and not just current tab matches. I think it would be quite useless if it would only use current tab matches (you have current tab matches in front of you anyway).

So instead of always returning a global result, it does search globally, and by means of an autocomplete give you global suggestions, but stays in the current tab, unless the users switches or when there are no more results in the current tab.

That said, if you're still not convinced, I'm more than willing to make changes!

We could also provide an option that allows the user to limit the filter to the current tab, a toggle.

As I said above, I'm not a big fan of an explicit toggle. It does what you say by default, but goes looking for results in other tabs once the current tab does not have any matches anymore.

I do like how the Vertical Tabs safely degrades back to fieldsets when JavaScript is disabled, unlike my current tabs implementation. I'm also concerned about the height of each tab. I like it except that when there are lots of tabs it requires lots of scrolling.

That is a matter of adding some CSS to make it look like your implementation. And I do agree! It's still a WIP :). What do you think of the current tab showing a list of enabled modules in the tab? Needed or not? Or just, as you have now, a number saying how many modules are enabled out of the total number of modules in the tab.

I'm hesitant to rely on a separate module in order to filter. I'm more willing to make the module dependent on it than have it optional. I'd be afraid of confusion when installing "Module Filter" and there is no "filter".

Easy switch to make, and I don't have a strong opinion about that.

In the current tabs implementation, when a tab is selected a hash tag is set in the URL. This allows any particular tab to be opened on page load, and potentially provides a forward/backward history (currently not working). Potentially the form can redirect to the the URL corresponding to the active tab when the form was submitted.

I already have a click handler in place which responds to tab clicks. It would be relatively straight forward to add the hash-tag stuff to my code, but I'm actually hesitant about it. I'd rather make it an option.

My final thought for now, what is the performance difference? Simply comparing JavaScript code, Kiphaas7's is a fair bit longer (not including lines of code from Instant Filter). Without actually performing tests, I can only assume the client takes a bigger hit on performance with the proposed changes. Module Filter is often used in dev environments where it's not uncommon to have hundreds of modules available, putting stress on any filter's performance.

Yes, the filtering happens more on the client side, and performance also heavily depends on the browser (version). Fortunately, all modern browsers (havent tested IE9 yet) handle my test case very well. That said, both my code and Instant filter go to great lengths to having results cached. Having simple loops of ~1000 items are done under 1 millisecond in modern browsers, it's the operations inside the loop that put strain on execution time. But the cache helps with that.

My personal dev site has about 100 modules divided in 16 categories. Due to a bug in Instant filter, for a long time there was a double loop going on, effectively making this 200 modules in 32 categories. I personally did not find any noticeable slowdown between entering the results and getting the results on screen, and I consider myself pretty sensitive in noticing slowdowns. I tested it in various current browsers, and the only one where I noticed something is IE8, which is to be expected given IE8 and below sucky JS performance. Still, workable. I even tried it on an iPhone 4. Got about a second delay after entering the first two letters, after that, smooth.

Kiphaas7’s picture

FileSize
399.95 KB

Arrgh, the image...

Kiphaas7’s picture

FileSize
19.77 KB

One more...

In the attached image, the asterisk next to the tab name indicates that changes were made to modules in the tab, but not saved. (UI pattern common across Drupal core). The names of the changed modules are however, updated in the tab summary (aggregator and block).

The green and red color of the table rows indicates the newly enabled/disabled state of the modules. But that is similar to the current module filter.

LarsKramer’s picture

Thanks to both for your efforts. So to review this, I need to download the ZIP in #14 and also install Instant filter?

Just a thought: if this is not a patch of Module filter but a whole new module (with the same name), maybe it would be better to continue work in a sandbox, provide a link here, and then later see if it could integrated? I will be glad to do testing and review.

Personally, I like the current feature of an "All" tab showing all modules in alphabetical order and being able to filter that list to for example only disabled modules, but I am open to changes (as a user).

klonos’s picture

What do you think of the current tab showing a list of enabled modules in the tab? Needed or not?

...I personally didn't like it that much, especially when the current tab has lots of enabled modules. Speaking of default core patterns/behavior:

...the asterisk next to the tab name indicates that changes were made to modules in the tab, but not saved. (UI pattern common across Drupal core)...

...the default vertical tabs behavior is to display a summary of enabled settings for all tabs, but that in our case would be terribly messy and would make the tabs huge (height-wise I mean). Anyways, we can either make it optional through some checkbox setting in the module's configuration or simply document in the module's README.txt that one can hide this additional info by using this css code in the theme level:

span.summary .module-fieldset-count,
span.summary .module-fieldset-list {
  display: none !important;
}

This hides the current tab's enabled modules count and/or list respectively - no biggie & it works fine for me.

In my post in #20 I suggest:

How about hiding tabs without results altogether instead of displaying 0/x?

I tried to figure a way to achieve this myself, but since one simply cannot traverse the DOM backwards with css selectors, I could only possibly do this with jQuery magic. So, can we please at least move the "no-matches" & "has-matches" classes to the parent <li> tags instead of the (grant-)child <em> tag?

Kiphaas7’s picture

@Larskramer: yes, preferably a dev version of instant filter. I recently took co-maintainership of that module, and I'm applying patches/improving it as I go. Most of the times when I find something in the module filter code. As long as I'm working on the module_filter code and keep refactoring the javascript, it's probably best to get the most recent dev of Instant filter.

As for a sandbox: actually, I should have done that earlier. http://drupal.org/sandbox/Kiphaas7/1256298 . Will push code ASAP.

@Klonos: you can post issues over there now ;).

Kiphaas7’s picture

FYI: in the mean time I managed to add a settings page, which contains toggles for an "All" tab, history entry for each tab click, autocomplete and for displaying the count/list of enabled modules.

Furthermore, Instant filter is now a dependency, and the tab heights are more like the current module filter (trimmed down each tab roughly 10 pixels).

@greenSkin: I think that covers most of your concerns? If you have more, or if I forgot a few, let me know.

klonos’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev

We do have a 7.x-2.x branch now and I gave it a spin after James requested a test in #1247484: Remember entered filter across tabs.

Still, I returned back to using the forked Module filter NG because it implements what we discussed here plus a lot of extra features (namely it hides tabs without results and auto-switches to the one with the most plus a few other really useful things).

With the original 7.x-2.x I did like the idea of having the module description occupy the full length of the table cell, but I didn't like the way the actions links got rendered because they got mixed with the requirements/dependencies sections (but then again this might only be an issue with my custom theme - see attached screenshot to understand what I'm talking about).

The one thing I really liked in the official 7.x-2.x version is the floating tab that makes the submit button stay always in view. Well though and implemented! I have a minor issue with this though, since my theme has a floating breadcrumb (to aid users with navigation and reduce scrolling in lengthy pages) plus the admin_menu module enabled which is floated too. This lead to a bit of a mess with floating elements overlapping each other, so I was thinking that perhaps instead of floating the tab with the submit button to the top/bottom of the page, we could have it float in the middle of it (height-wise I mean).

Anyways, I need to say that I seem to be "addicted" to the Module filter NG forked sandbox and I really-really need to know if the features implemented there will eventually find their way back to the original project here. If not, I thing it should become a complete project of its own. We need an answer on that from the maintainers of this project, so I asked James to drop a line here and let us know.

klonos’s picture

...and here's the screenie I mention in #31 ;)

greenSkin’s picture

@klonos I've addressed the issue in 7.x-2.x you mentioned in post #31. The links now appear at the bottom, below the "requires" and "required by" sections.

I've incorporated various ideas from Kiphaas7's sandbox in the 2.x branch. Though, I am still not convinced of the vertical tabs and the reliance of InstantFilter to do any filtering (even with a dependency). If/when InstantFilter or similar functionality is in core I will definitely utilize it.

A global filter, I believe, is the biggest need for Drupal's modules page. An "all" listing or filter result fits that need the best I think. With vertical tabs that really isn't an option aside from the autocomplete functionality Kiphaas7 implemented. Previous versions of Module Filter had an autocomplete feature but realityloop and myself felt it was redundant to what was already presented via the "All" tab. This of course isn't the case when using it with core's vertical tabs since an "All" tab or list is simply not feasible. The autocomplete does counter the limitation to not being able to present a global result, allowing the user to still easily find the module they are looking for. I do have to state though that I think a global filter result provides better usability, it eliminates the step of selecting the module from the autocomplete list just to then be able to interact with the row. Also, it can be quite convenient using the sub-filters (enabling, disabling, etc.) with a global result. The user can see ALL modules meeting those sub-filters without having to jump around within tabs.

I've tried to theme the tabs in 2.x to look more like core's vertical tabs so at least from the frontend it gives a more consistent look across the whole Drupal site.

I hope I've explained my feelings adequately. If not, feel free to inquire.

I would like to take a moment to mention a couple other features implemented in the 2.x branch.

  • By default the list shows all modules when no tab is selected. When no tab is selected the list acts like the 1.x branch's "All" tab. You can mouse over the rows and see a subtle highlight of the tab the module resides in. Select a tab and the filter results limit to the active tab. Click the active tab and it deselects.
  • When checking/unchecking modules the visual aid has changed. Now the module to be enabled/disabled has it's name shown in green or red accordingly within it's tab. The 1.x only presented a green/red count. I'd like to still have a numeric count but as a grand total that is presented by the submit button.
  • Inspired by Kiphaas7's work, when filtering, each tab presents the count of modules it contains that matches the filter. There is an option in settings that allows tabs with no results to be hidden (for you, @klonos :)).
  • The Updates report page now is filterable. Still under the context of filtering modules but graciously extends the filter on this page to include themes and core.

@klonos you mentioned the desire to move features from the sandbox back into the main project. I'm assuming I've now done that? Of course minus the use of InstantFilter, core vertical tabs, and an autocomplete textfield.

Kiphaas7’s picture

greenSkin, that sounds great. However, I hope you respect my decision to continue my work on an implementation based on Instant filter. I will naturally mention Module filter as an inspiration, where applicable.

greenSkin’s picture

@Kiphaas7 I respect your decision. The work you've done provides a sound alternative to Module Filter.

greenSkin’s picture

Status: Needs review » Closed (works as designed)