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

fixing a select option should remove all the others. #3097

Closed
Tracked by #1245
johrstrom opened this issue Oct 3, 2023 · 8 comments · Fixed by #3159
Closed
Tracked by #1245

fixing a select option should remove all the others. #3097

johrstrom opened this issue Oct 3, 2023 · 8 comments · Fixed by #3159
Milestone

Comments

@johrstrom
Copy link
Contributor

johrstrom commented Oct 3, 2023

When you fix a select option - it should highlight the fact that you've removed all the other options. Functionally I don't think it makes a difference if the exclude_options is populated because fixed is true - I just think it's an issue of uniformity in experience.

It should likely go the other way to. When you remove all but 1 select option, it should set it to fixed.

@johrstrom johrstrom mentioned this issue Oct 3, 2023
45 tasks
@osc-bot osc-bot added this to the Backlog milestone Oct 3, 2023
@abujeda
Copy link
Contributor

abujeda commented Nov 1, 2023

I have been doing some analysis and prototyping on this.
I have implemented 2 approaches:

1 .- Disabling all options when fixed is selected:
fixed_fields

2.- Emulating remove other options, but without updating the exclude list:
fixed_fields_2

@abujeda
Copy link
Contributor

abujeda commented Nov 1, 2023

As fixed and excluded attributes control different configuration settings, it feels that disabling all (option 1 above) is more consistent with the implementation.

@johrstrom
Copy link
Contributor Author

2.- Emulating remove other options, but without updating the exclude list:

I like number 2 better - just from a visual/UX perspective. It makes me as the user really understand what's happening.

That said - it appears you do update the exclude list (even if we ignore it at later points). I get your concern around implementation, but I guess I'd have to give UX precedence here.

@abujeda
Copy link
Contributor

abujeda commented Nov 1, 2023

OK - will continue with option 2

it appears you do update the exclude list....

In the current implementation, the exclude list is never updated when fixed is selected. I am just replicating the UI changes for the non selected items. But it will be easier to see once I submit the first draft PR.

@johrstrom
Copy link
Contributor Author

In the current implementation, the exclude list is never updated when fixed is selected. I am just replicating the UI changes for the non selected items. But it will be easier to see once I submit the first draft PR.

OK - I guess I would question what happens when you fix, save, reload then unfix, then save again.

I guess I'm just looking for what you'd expect to happen to happen when you do that (fix, save and then un-fix).

@abujeda
Copy link
Contributor

abujeda commented Nov 1, 2023

fix, save and then un-fix

At the moment, the exclude list will not be updated when using fix. So when you fix, save, and un-fix, It should render the UI using the last saved exclude list to disable those elements in the select dropdown.

Do you think that the exclude list should be "deleted" when fix is selected?
So that fix, save, and un-fix will simply start with an empty exclude list?

@johrstrom
Copy link
Contributor Author

Do you think that the exclude list should be "deleted" when fix is selected?
So that fix, save, and un-fix will simply start with an empty exclude list?

I would guess the opposite. I mean my north star is what the user would expect to happen. As a user I'd expect when I un-fix the exclude list excludes everything besides the value I had just fixed to.

In any case - it seems like this is an edge case we can account for later. We can pick that up in another ticket if it gets too complicated in this one.

@abujeda
Copy link
Contributor

abujeda commented Nov 2, 2023

created an initial PR: #3159

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 a pull request may close this issue.

3 participants