Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I'm thinking of using this module for one of my projects. Don't know if its gonna make it, but I needed some extra functionality and I coded it in the two following commits/patches.
Please check them and if you like/approve them merge them :)
Comment | File | Size | Author |
---|---|---|---|
#12 | option_to_enable_module-2509488-12.patch | 3.86 KB | rcodina |
Comments
Comment #1
bserem CreditAttribution: bserem as a volunteer and at SRM for SoLebIch commentedComment #2
bserem CreditAttribution: bserem as a volunteer and at SRM for SoLebIch commentedComment #3
bserem CreditAttribution: bserem as a volunteer and at SRM for SoLebIch commentedComment #4
rcodina CreditAttribution: rcodina commentedThanks @bserem. I will check this out ASAP. I like the idea of having the option to choose between blacklist or whitelist. However, I don't see why users would want to have this module enabled on non admin pages.
Once said that, I don't like the idea of renaming the variable "vertical_tabs_responsive_path_blacklist" to "vertical_tabs_responsive_path_list" because this forces me to think about an upgrade path (delete old variables). So I would appreciate a new patch respecting this variable name or a new patch with an upgrade path.
Comment #5
bserem CreditAttribution: bserem as a volunteer and at SRM for SoLebIch commented@rcodina I agree with the variable naming.
As for the non-admin paths. In my case, we use vertical tabs on the user profile page, and do not want to provide visitors access to the theme (seven).
Like I said in my first post, these patches are an idea/work to see if this module fits our needs. I had to adapt it a bit, and these are the results. Feel free to use anything you prefer from the above.
In the next days I'll be called to find a solution about vertical tabs on our site, lets see how this goes.
Comment #6
rcodina CreditAttribution: rcodina commentedOk @bserem, then I will wait until you find out if this fits your needs. Then, I will test this, maybe do some changes and finally commit it with you as author.
Thanks for your interest in this module. I hope to keep improving it with community help!
Comment #7
rcodina CreditAttribution: rcodina commentedComment #8
tchopshop CreditAttribution: tchopshop commentedI'd MUCH rather have whitelisting than blacklisting... Only allowing blacklisting means the module shows up in unexpected locations. It's more likely to be needed in one or two spots rather than the whole site.
thanks!
Comment #9
bserem CreditAttribution: bserem as a volunteer and at SRM for SoLebIch commentedWe went with a custom JS option, because we needed some fine control over our tabs. So, I didn't spend much more time this module.
However, you can build upon my patches to improve whitelist if needed.
Comment #10
rcodina CreditAttribution: rcodina commentedAs soon as I have some time I will work on this. Today I begin my three weeks vacation time ;)
Comment #11
tchopshop CreditAttribution: tchopshop commentedThank you... have fun!
Comment #12
rcodina CreditAttribution: rcodina commentedHere comes my patch which solves both requests by @bserem. Please review it.
Comment #13
rcodina CreditAttribution: rcodina commented@tchopshop Please, test it too.
Comment #14
rcodina CreditAttribution: rcodina commentedI have done some testing and it works for me.
Comment #15
rcodina CreditAttribution: rcodina commentedComment #17
rcodina CreditAttribution: rcodina commentedPlease, test this. I don't want to create a new release with this without another positive review. Now this is on dev branch of module to make it easier to test changes. Many thanks!
Comment #18
rcodina CreditAttribution: rcodina commentedThis is now fixed. It is on brand new 1.0-rc1 version.