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

Added definition_order parameter to CrossSelector #570

Merged
merged 2 commits into from Aug 1, 2019

Conversation

jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Aug 1, 2019

Right now you can't specify any chosen ordering in the selected list on the right side of a cross-selector as the filtering forces the items into the definition order for options. Note that when you set the order via value, the widget temporarily accepts the chosen ordering before reverting to definition order.

In some situations, you want to specify the ordering of the selected list based on the ordering you add/remove items to/from it. This PR adds a definition_order parameter that you can set to False to allow this use case.

I'm not entirely sure that self._lists[selected].values is the right set of values here but it does seem to work correctly in my (limited) testing. At the very least I no longer see the reordering back to definition order.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Aug 1, 2019

Two comments:

  1. If you add options dynamically do we need special handling to ensure that this option is respected?

  2. I'd like to see a test.

@jlstevens
Copy link
Contributor Author

@jlstevens jlstevens commented Aug 1, 2019

If you add options dynamically do we need special handling to ensure that this option is respected?

I don't think so. Adding options dynamically seems to work though any selection is cleared (true before this PR as well).

I'd like to see a test.

Agreed.

@jlstevens
Copy link
Contributor Author

@jlstevens jlstevens commented Aug 1, 2019

To recap the behavior I observed prior to this PR (and that is still present):

  • Adding to the options dynamically resets any selection even if the selected items are members of the new options.
  • Shrinking options does seem to filter out the selection on the right side without deselecting them and moving them to the left side.

The former point is probably a bug but I think the fix is outside the scope of this PR.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Aug 1, 2019

Looks good. Happy to merge.

@philippjfr philippjfr merged commit 9363537 into master Aug 1, 2019
2 of 3 checks passed
@philippjfr philippjfr deleted the crossselector_order_option branch Sep 9, 2019
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