axa-ch-webhub-cloud / pattern-library

AXA CH UI component library. Please share, comment, create issues and work with us!
https://axa-ch-webhub-cloud.github.io/plib-feature/develop
125 stars 18 forks source link

Dropdown shows extra defaultTitle on mobile #2363

Closed salle18 closed 1 year ago

salle18 commented 1 year ago

@axa-ch/dropdown v12.0.1

Dropdown options on mobile screen always add default title. This issue is reproducible in the storybook.

Desktop/wide screen

Screen Shot 2022-11-03 at 10 06 28

Mobile/narrow screen

Screen Shot 2022-11-03 at 10 06 45

Expected Behavior

Options on both mobile/desktop are always the same.

Current Behavior

Mobile screen always adds default title to the list of options.

Steps to Reproduce

  1. On desktop open https://axa-ch-webhub-cloud.github.io/plib-feature/develop/?path=/story/components-dropdown--dropdown
  2. Select amount and open dropdown again
  3. Default title select amount is not visible
  4. Open inspector and enter mobile view
  5. Open dropdown again
  6. Default title is visible

Context (Environment)

Cosmetic/consistency on mobile devices, allows user to select invalid empty value on the overview.

Possible Solution

defaultTitleIfNeeded should not add defaultTitle if there is anotherSelection (already selected item).

MKaHead commented 1 year ago

Hi @salle18, We'll take a look at it right away. At first glance, I noticed something about the native select end that made me wonder.

MKaHead commented 1 year ago

@salle18 I'm still waiting for design feedback here, whether I should fix it uniformly, or in my personal opinion, that we should remove the defaultTitle. As it only confuses from enduser UX, it just disappears item. If you really want to have it, you can also implement it yourself via the items.

The reason why this was built in seems to be legacy, see readme: "Its intended use is primarily for native-HTML situations where server-generated items describe the choices proper, and a separate title like defaulttitle="Please select" prompts the user to make a choice."

MKaHead commented 1 year ago

@salle18 I have now received feedback from designers/UX and they have told me that we are taking out the defaultTitle feature, which will be a breaking change, so there will be a new major version. We recommend to pre-select default item or create an item for example "Please choose".

I hope this is not too disappointing. Thanks for reporting!

markus-walther commented 1 year ago

@salle18 @MKaHead In my opinion, the real issue is, as you write above that it "allows user to select invalid empty value on the overview.".

That is, in narrow viewports where the native select-based dropdown will be shown when dropped down, the defaulttitle option is selectable, where it should be purely decorative. HTML provides the means to make it non-selectable https://developer.mozilla.org/en-US/docs/Web/HTML/Element/option#disabled, and this was in fact used in the code.

But: the code had a typo, so it did not work. Now, as a consequence of fixing https://github.com/axa-ch-webhub-cloud/pattern-library/issues/2416, the selectability bug is also fixed here, as of npm version 12.0.4.

In narrow viewports where the native select-based dropdown will be shown we basically have to use an <option> item as a carrier of the defaulttitle, though, and Pattern-Library-internal consumers of the axa-dropdown component such as axa-datepicker do use that functionality.

Hence, for now it seems the feature-request part of this issue cannot be adressed without undue effort. But the bug-fixing part can and has been, which arguably is already a big win for everybody.