Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add All and Remove All buttons for the select_advanced dropdowns #1331

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

brianlayman
Copy link

This code is functional for the site I am working on. So I figured I would submit it for a more expert review.

Basically it allows you to define a bulk_clone boolean parameter for the advanced select dialog. Then in the association editor ( I'll submit a pull request over there too), you can add all possible associations or remove them.

I am unhappy with the JS code that calls mousedown() event to remove the items and then to re. I would much rather trigger the method to remove an item directly without causing the drop down to open, but the minimized select2 code blocked my plans for that. For now. That said, since it is fully functional, perhaps the way it is written is actually good enough to deploy.

It may be that this code could be used for other field types as well. I've only done testing with the select field in use with association of taxonomies. I've not put any thoughts into whether this could work for another type of field.

Please let me know if you think this code needs any alterations.

@rilwis
Copy link
Member

rilwis commented Sep 3, 2019

Hi @brianlayman ,

We already have select_all_none option for choice fields. Is this a duplicated functionality?

@brianlayman
Copy link
Author

brianlayman commented Sep 3, 2019

@rilwis No, I don't think so. It is similar, but I don't think this is quite the same. Maybe you can confirm this for me.

"select_all_none" works ONLY with a choice/select field set to "multiple" mode. It allows you to choose all or none of the options in a single drop down. Right?

With the association editor, the drop down has "multiple"=False and "clone" = true. It is a bunch of dropdowns cloned in a single metabox. This allows not only the full list of selections to be easily seen at once, but also allows you to order the selections. Ordering is not something not possible, AFAIK, with "multiple"=true. So this Add All/Remove All is different functionality.

I think it would be possible to have both options active at once. This adds an additional use case I should code for and test. I hadn't thought about the condition of clone=true AND multiple=true so this code won't handle it as is. Frankly, I'm not sure what that would produce in the database. Is it even a working configuration?

@brianlayman
Copy link
Author

Note that the ajax change broke the JS portion of this. I was using the options in the drop down to populate the selections. Since not all options are in the drop down, not all options are currently added.
I'll fix this and resolve the conflict in the CSS file today, if possible..

brianlayman pushed a commit to brianlayman/mb-relationships that referenced this pull request Oct 30, 2019
Because query_args is now a sub-array under fields, it too needed to be merged with the defaults.
Added code to prevent ajax being active by default (commit wpmetabox@7d91f9b ) with the bulk_clone feature of wpmetabox/meta-box#1331
@brianlayman
Copy link
Author

The resolution of the ajax difference was handled entirely in the Relationship plugin and not here.
So the only required change here was to resolve the conflict created when both this PR and master added lines to the end of css/select-advanced.css

Additionally the commit I did included lossless optimization of the image files included in this plugin. As there are no visual changes to the image, I've included them in this commit. All of the changes are small, but for example ui-bg_flat_0_aaaaaa_40x100.png went from 180 bytes down to 86 bytes.

The most recent commit fixed one place where I referenced a filter named add_all... and it should have been remove_all...

Here is the demo of the feature:
https://www.screencast.com/t/ZPr70d8oen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants