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

Component: Single-Select using ChoicesJs +Multiselect #328

Merged
merged 70 commits into from
Aug 25, 2023

Conversation

verena-ifx
Copy link
Contributor

@verena-ifx verena-ifx commented Jul 28, 2023

By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---

CONTRIBUTING.md also tells you what to expect in the PR process.

Description
Single Select using ChoicesJs library
Mult select (own development)

📦 Published PR as canary version: 20.9.0--canary.328.b992a154c134c1d23bb428d3d4ff957571709a74.0

✨ Test out this PR locally via:

npm install @infineon/infineon-design-system-stencil@20.9.0--canary.328.b992a154c134c1d23bb428d3d4ff957571709a74.0
# or 
yarn add @infineon/infineon-design-system-stencil@20.9.0--canary.328.b992a154c134c1d23bb428d3d4ff957571709a74.0

@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2023

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-08-25 13:15 UTC

@verena-ifx verena-ifx added the minor minor version bump label Aug 11, 2023
@github-actions
Copy link
Contributor

--STORYBOOK-PREVIEW--

@verena-ifx verena-ifx added this to the Milestone 1 milestone Aug 14, 2023
@verena-ifx
Copy link
Contributor Author

e2e tests missing

@tishoyanchev
Copy link
Contributor

tishoyanchev commented Aug 22, 2023

Select:

  • why not group both single and multi into a single Select in Storybook? because they are separate components, one with library, one without. its not possible to have them in one story
  • how should the multi-select work with the Chip component? that is not needed for milestone 1

Single Select:

  • when click the select second time to close menu, the arrow remains up

  • arrow is not vertically centered inside the field

  • size value is incorrect. It should be 'm' or 's', not the full label 'small (36px)'. In Storybook, it should be 'm' and 's' but with the exact size with brackets. Eg: s (36px). But this is only the control label, the value is still 's'.
    Verena: we discussed this with Sergej and he said we should use that in the future
    Tisho: To me this makes no sense. It is inconsistent with literally every other component we have, and it's difficult to set. This is also not a Designer's decision. The idea we talked about before was to have the pixels in brackets in Storybook, not to have size value be renamed to literally 'small' and 'large' with the pixels included in the naming too. How would users know what's the pixel size anyway? They just know small, medium or large.

  • error message and label value overflow will check this
    Use this for the overflow:
    white-space: pre-wrap;
    word-wrap: break-word;
    overflow-wrap: anywhere;

  • value property is missing in storybook
    Verena: thats the pre-selected value you can set if you dont want a placeholder to show, the value needs to exist in the provided options of course
    Tisho: then why not make one take preference over the other? For instance, if 'value' is set, it replaces the placeholder even if it's set. Currently if you have a placeholder, and you write something to 'value' other than 'undefined', you get an error, and the entire component is not showing without explanation for what's happening. There should be no error. One should just take preference.

  • what do I have to pass in value? If I write a string, I get an error, and component not showing.

  • There's a lot of commented out code. will remove this

Multi Select:

  • still have the weird symbols that escape single quotes.
  • arrow is not vertically centralized
  • should the selected option not be wrapped in a chip-like wrapper just like type multi of single-select?
  • label and error message overflow
  • same issue with the size as single-select

@verena-ifx
Copy link
Contributor Author

  • arrow position fixed
  • label+error msg overflow fixed
  • options array in story now beautified

@verena-ifx
Copy link
Contributor Author

e2e tests added

@tishoyanchev tishoyanchev merged commit b9d9bec into master Aug 25, 2023
8 checks passed
@github-actions
Copy link
Contributor

🚀 PR was released in v20.9.0 🚀

@verena-ifx verena-ifx deleted the 157-dropdown branch October 9, 2023 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor minor version bump released
Development

Successfully merging this pull request may close these issues.

2 participants