-
Notifications
You must be signed in to change notification settings - Fork 684
When source data has display:none, hide the options #674
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
Conversation
| (function($, undefined) { | ||
| // http://stackoverflow.com/a/3640212/401636 | ||
| // custom css filter ex: :css(display=none) | ||
| $.expr[':'].css = function(obj, index, meta, stack) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not fond of modifying anything in jQuery for this widget. This is a side effect that could impact other defined expressions in other widgets or by the user.
You should declare a method in the widget instead and use that as a parameter to filter.
|
@mlh758 I am not clear what the ultimate objective of this PR is. It seems like it is duplicating the filter functionality. Is the intention to ignore options on the native select that are hidden? If so, I am a bit unclear on the benefits of that.... If there are options in the select that the widget should not use, it begs the question of why the "dead options" are there in the first place. If we cannot be sure of the objective and we cannot see a clear benefit, then this PR should be closed. |
|
The objective is to have the widget work, when some of the options have been hidden. Without this patch, the cache indexing was fouled up when filtering the available options. As a User with lot of options Is this what #745 achieves? |
|
@aarcro Still not clear... which options? Options in the underlying native select? I a rewriting the filter stuff now, anyway--#745 was just an "initial salvo". Going to a class name based toggling scheme (w/ associated CSS rule) instead of directly showing/hiding, which should better allow us to separate out what the widget is doing vs not. cc @mlh758 |
|
I think I was hiding elements in the native select from the server side to set the initial state of the filter. My memory is just that I had to tweak both the visible widget and the native to keep them in sync and ensure that choices made while filtering were still on the correct items when removing the filter. I'd expect that the class name approach would resolve this as well. |
|
@aarcro @mlh758 closed this PR. If you want me to drop you a follow-up when I get to a workable PR, I would suggest reopening this so I do not forget to notify you. One thing to consider is that the cache has been built off of the native select options in the past with no consideration of option group labels, which prevented them from being searched. I am addressing that, also, although I am now building the cache from widget options instead. Does it matter to you whether I build the cache from the native select options or the widget options? It should not matter if the two are in sync, but I am trying to avoid going back to the native select once the widget options are created via refresh. Any thoughts? |
|
Yes, searching option group labels was likely what I was addressing. I do have many option groups and want to filter on their labels :) No need to reopen, if/when I get pack to that project I'll see the link to this PR and follow the thread. +1 for not going back to the native Thanks! |
When there is a long list of choices, it is valuable to show a checkbox that can hide some of the options. To be able to later show them again in their original location, the simplest approach is to use
.hide()(which sets an inline style ofdisplay:none) on the<option>elements.By looking specifically for this style (rather than
:is_visible), we can hide these elements in the widget when callingrefresh()The filter addition must also ignore these elements when rebuilding it's cache
Sorry, I don't know how to create tests in javascript, but I'm using this small modification so I can hide/show various options in the widget by show/hiding the original data elements.