search

ManageIQ / manageiq-ui-classic

Classic UI of ManageIQ
Apache License 2.0
50 stars 357 forks source link

Weird UX when adding tags to a group #5553

Closed himdel closed 4 years ago

himdel commented 5 years ago

Cog > Access Control > Groups - Edit a Group

under the My Company Tags, we have the new tagging editor now, as of https://github.com/ManageIQ/manageiq-ui-classic/pull/5482

The behaviour is ...rather surprising though...

You get two dropdowns, the first choses the category, that's fine - but choose a category without the (i) icon. The second dropdown doesn't behave like a dropdown at all:

Expected: you should only see the later item Actual: both items get added once chosen (unless the category only allows one).

I think the dropdown kinda makes sense for categories where only 1 item can be chosen, but we should have a multiselect for the other kind.

This is probably for you @terezanovotna , @epwinchell , @PanSpagetka Cc @romanblanco

epwinchell commented 5 years ago

cc @skateman

romanblanco commented 5 years ago

I think the dropdown kinda makes sense for categories where only 1 item can be chosen, but we should have a multiselect for the other kind.

Agree with @himdel. The selection for multiple items should be done as multiselect, indicating already selected items

Screencast from 2019-05-10 12-11-32

@PanSpagetka Also, there are multiple "Save" buttons on this screen (the same as https://github.com/ManageIQ/manageiq-ui-classic/pull/5482#issuecomment-486586333)

PanSpagetka commented 5 years ago

I agree that it would be better as a multiselect. Given the time I have spend with this I can't really tell if it is weird or not. But it should work like this, items without (i) can have multiple values selected.

@romanblanco I believe, that if you update your setup you will find only one set of buttons.

PanSpagetka commented 5 years ago

After discussion my plan is following:

  1. Wait for https://github.com/ManageIQ/react-ui-components/pull/109 to be merged. Tests are passing, I am not sure if there is anything what needs before it can be merged.
  2. Use multiselect when multiple values are possible (without (i))
  3. Get some tea :tea:
  4. Change order, third point should be zeroth(?)
martinpovolny commented 5 years ago

Wait for ManageIQ/react-ui-components#109 to be merged. Tests are passing, I am not sure if there is anything what needs before it can be merged.

Done. Feel free to ping me next time.

miq-bot commented 4 years ago

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

h-kataria commented 4 years ago

looks like fixed in https://github.com/ManageIQ/react-ui-components/pull/109