Needs work
Project:
Improved Multi Select
Version:
7.x-2.x-dev
Component:
Code
Priority:
Major
Category:
Feature request
Assigned:
Reporter:
Created:
14 Oct 2015 at 14:21 UTC
Updated:
17 Feb 2017 at 08:07 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
dmitrii commentedGreat work @koosvdkolk, thank you!
I've tested your patch and in general it works.
But there are some problems.
The behaviour of the remove all button is different now. It should removes all visible items but it removes all items, including hidden by filter.
Also there are some issues related to the Drupal coding standards, but it's minor and can be fixed in any IDE by the code auto format.
What do you think about adding configuration option to enable your functionality? It seems to me it should be disabled by default to prevent UI problems for users who will update the module from earlier versions.
Comment #3
dmitrii commentedComment #4
jvdkolk commentedThanks! I will dive into it.
To be honest: In my own implementation I have removed the 'move all' options, because my users don't need them. Can't we make these optional as well?
Comment #5
dmitrii commentedI think it's a good idea. We can implement something like this for the add/remove all buttons: if label is not set - hide it from UI.
Comment #6
dmegatool commentedThat's some really nice additions. I just installed Improved Multi Select for the first time and the lack of shift+click struck me immediatly. Can't wait to see this integrated in the next release.
Comment #7
wolffereast commentedFixed the remove all also removing hidden items issue.
Also forced a re-filter on both the add and remove buttons.
I picked off any coding standards issues I cam across, though I didn't run it through pareview so I probably missed a few. I am going to take a wack at adding configuration options for the add/remove all buttons as well as the new filtering functionality when I get a chance.
Comment #8
wolffereast commentedWhat are your thoughts on moving the button click functions into their own named functions instead of using anonymous functions in a click bind? I think that might make some of these changes a bit easier, especially if some of the functions need to change depending on the setup. It will also clean up the attach function significantly, making this file a bit easier to work with when making updates.
Comment #9
wolffereast commentedI added two new admin options. The first is a checkbox that turns the add/remove all options on and off, which defaults to on. I also added a checkbox that turns the new secondary filter on, which defaults to off.
I split the css for the new filters out so that the single filter looks the same as it used to.
I broke the js click handlers for the add/remove buttons out into their own named functions.
Comment #10
wolffereast commentedMoved the other click binds into named functions for good measure (just to be consistent). I know much of this might need to be in a separate patch to clean up the module's commit history, but we can always split it up if need be.
Comment #11
wolffereast commentedComment #12
jvdkolk commentedWorks fine with me
Comment #13
wolffereast commentedForgot to clean up after myself... The only difference between this patch and the previous one is the addition of 2 variable_del calls in the uninstall hook. Leaving at RTBC
Comment #14
WorldFallz commentedI'd love to get these taken care of, but we should really do it in separate patches. Feel free to update if anyone gets to it before i do.
Comment #15
wolffereast commentedSome of this functionality was originally pulled from other requests. I will compile a list of links and drop them here (along with updates to the patches in those tasks if appropriate) when I have a chance
Comment #16
jvdkolk commented@wolffereast: any news on this issue?
Comment #17
wolffereast commented@koosvdkolk Sorry, been very busy lately. I think this will need to be deconstructed into several patches as I was unable to find the original issues I thought existed. I must have been mistaken, anyways I think an initial patch that updates the JS structure without changing anything functionally followed by individual patches for the different improvements is a good idea.
I will try to put eyes on this soon
Comment #18
jvdkolk commentedThanks!