Extending on the changes made in #2051119: Field's minimum width should be passed through options of the chosen instance, I have introduced some extra configuration for the minimum width setting that allows users to have more control over how they want their Chosen field to appear.
For my particular use case, I needed to have it have a percentage-based width to support use in a responsive layout. Because of the inline style, this was a pain to do in custom css (required an !important flag which I just hate doing), so adjusted it here instead.
The changes just add an extra configuration option for minimum width units:
I had started down a track of removing the inline style altogether which I believe would be more theme-friendly than this, but understand that there are a lot of users who don't want to or can't tinker with the theme css themselves.
I also considered just lumping it all in the one field, but I think this way is a bit more noob safe.
It still defaults to 200px so nothing changes if you hadn't configured it yet.
Comment | File | Size | Author |
---|---|---|---|
#16 | chosen-responsive_width-2075579-16.patch | 4.39 KB | jstoller |
#8 | chosen-responsive_width-2075579-8.patch | 2.83 KB | skorzh |
#5 | chosen.code_.2075579-5.patch | 3.82 KB | Harlor |
#1 | chosen-add-additional-minwidth-settings-2075579-1.patch | 4.47 KB | marblegravy |
Comments
Comment #1
marblegravy CreditAttribution: marblegravy commentedComment #2
marblegravy CreditAttribution: marblegravy commentedRetrospectively, perhaps references to a minimum width should also be replaced with just 'width.' After all, that's what we're setting - the actual width. There's nothing much minimum about it!
Thoughts?
Comment #3
Hydra CreditAttribution: Hydra commentedGenerally I think this would be a good improvement! Wording and usability of course is an issue we also address in #2064325: Revise configuration page, perhaps we can put our thoughts here together.
Some little issues with your patch:
Renaming will cause the lost of previously saved data. We need an update hook to transfer this data to the new variable.
Missing new line of the end of the javascript file.
Besides this, the admin_settings_form is storing the chosen_width_settings in a variable, which needs to be deleted in chosen.install chosen_uninstall() (and of course the chosen_minimum_width one will be obsolete).
Comment #4
marblegravy CreditAttribution: marblegravy commentedThanks for the review - I'll try and have another crack at it.
Comment #5
Harlor CreditAttribution: Harlor commentedWell I converted the patch for the new dev version. I think there is an issue with the units in the chosen.js file.
This wont work for other units!
$(this).width() is always in px, minWdth is not.
Well I thing there is no possibility to convert tehse units... maybe we have to accept that for other units the setting is not the min setting but the absolute setting.
Comment #6
Hydra CreditAttribution: Hydra commentedWell patch looks good so far to me, except the unit thingy :/
One possible solution would be to seperate px from other units in the comparison. So if unit == px, we make the minWidth check. If not, the value will be used as fix width for all select fields.
I would like to postpone this till the conversion to plugins has been made. Perhaps while creating a generic way to implement plugin-based options, we find a practical solution. #2076813: Move library options (configuration) to library plugin
Comment #7
MrPaulDriver CreditAttribution: MrPaulDriver commentedAny update on this
Comment #8
skorzhThank you guys for your patchs, but it doesn't work with current version of module.
I created a new patch for this problem, I didn't test it for all possible issues, but seems it works for me well.
Comment #9
gmclelland CreditAttribution: gmclelland commentedThanks. Patch in #8 works for me, but it did have some whitespace errors.
Comment #12
dddbbb CreditAttribution: dddbbb commentedWhile the patch in #8 works in that it does what it sets out to do, I still don't think that this is the right solution: We're still left with an inline width style on the chosen-container div and overriding this from the theme still requires the use of !important in stylesheets (not good).
I think that we should be removing the inline style altogether or at least providing a config option for this.
Comment #13
dddbbb CreditAttribution: dddbbb commentedLooks like this kind of approach is in the works in the Chosen.js library: https://github.com/harvesthq/chosen/pull/2091
Comment #14
joelpittet@danbohea, I agree. Maybe removing the requirement of this and setting it to empty would suffice with the latest chosen library?
Comment #15
joelpittetMade a patch for that over here to not disrupt this one #2790897: Let the width be flexible
Comment #16
jstollerI rerolled #8 to work with the latest 7.x-2.x-dev release, making the following changes:
chosen_width
andchosen_width_units
chosen_width
, to make sure it's numeric