feat(select): add multiple option to limel-select
#350
Conversation
…h ellipsis instead
… scrollbar if content is longer
ghost
added
the
adrianschmidt
force-pushed
the
multiple-option-for-select
branch
2 times, most recently
from
1f7fd41
to
b50470f
Compare
…multi-select BREAKING CHANGE: `limel-multi-select` has been removed. Use `limel-select` with the `multiple` attribute instead. fix #203
adrianschmidt
force-pushed
the
multiple-option-for-select
branch
from
b50470f
to
75f8baa
Compare
| .map(option => { | ||
| return option.text; | ||
| }) | ||
| .join(', ') |
There was a problem hiding this comment.
This would be more readable if it was refactored into a function, e.g. getHeader
| ); | ||
| } | ||
|
|
||
| private renderCheckbox(index: number, option: Option) { |
There was a problem hiding this comment.
We should try to reuse as much code as possible and there seem to be a lot of similarities between this and both the checkbox and list components. Can we not reuse those instead?
There was a problem hiding this comment.
This is just a straight copy-paste from the old limel-multi-select. Of course we should improve on it in future iterations, but we needed something quick to make the add-new dialog in the webclient releasable. Perfect is the enemy of good.
| public render() { | ||
| return ( | ||
| <div class="multi-select"> | ||
| <limel-collapsible-section |
There was a problem hiding this comment.
Did it seem to be a lot of work to get this working with limel-menu instead?
There was a problem hiding this comment.
The menu kept closing when anything was clicked and I couldn't find an immediate solution so I went with something I could finish before going on vacation.
src/components/select/select.tsx
Outdated
| @@ -34,93 +40,41 @@ export class Select { | |||
| @Element() | |||
| private limelSelect: HTMLElement; | |||
There was a problem hiding this comment.
This one is no longer in use
| shadow: true, | ||
| styleUrl: 'select-multiple.scss', | ||
| }) | ||
| export class SelectMultiple { |
There was a problem hiding this comment.
Since this component and SelectSingle are "internal" and should not be used anywhere else, maybe we could move them to a subfolder of select if Stencil allows that without complaining
There was a problem hiding this comment.
Stencil tends to complain. I haven't tried it. I'm going on vacation for almost two weeks in a while, but you are welcome to try it.
fix #203