From 2f9be81656d160ee16626f1575ebd81463188ce7 Mon Sep 17 00:00:00 2001 From: Ang Dawa Sherpa <89925822+angsherpa456@users.noreply.github.com> Date: Thu, 4 Jul 2024 12:02:00 +0200 Subject: [PATCH] fix(ui-library): remove slot from select component (#1131) --- package.json | 1 + packages/js-example-app/src/index.server.ts | 15 +- .../src/components/select/index.css.ts | 4 - .../src/components/select/index.stories.ts | 206 +++++++----------- .../src/components/select/index.test.ts | 29 +-- .../ui-library/src/components/select/index.ts | 39 ++-- .../src/components/select/renderFunction.ts | 5 +- 7 files changed, 116 insertions(+), 183 deletions(-) diff --git a/package.json b/package.json index c9d9a78c4..4d76284f8 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "types": "", "scripts": { "start:storybook": "yarn tokens:generate && yarn compile:icons && yarn workspace @boiler/ui-library start:storybook", + "build:js-example-app": "yarn workspace @boiler/js-example-app build:dev", "start:js-example-app": "yarn workspace @boiler/js-example-app start:dev", "build:storybook": "yarn tokens:generate && yarn compile:icons && yarn workspace @boiler/ui-library build:storybook", "build:ui-library": "yarn tokens:generate && yarn compile:icons && yarn workspace @boiler/ui-library build && yarn workspace @boiler/ui-library compile", diff --git a/packages/js-example-app/src/index.server.ts b/packages/js-example-app/src/index.server.ts index 469e36378..d995c915e 100644 --- a/packages/js-example-app/src/index.server.ts +++ b/packages/js-example-app/src/index.server.ts @@ -61,14 +61,15 @@ export function* renderIndex() { aria-label="Select" select-id="selectId" name="select" + options=${JSON.stringify([ + { label: '--Please choose an option--', value: '' }, + { label: 'option 1', value: 'option1' }, + { label: 'option 2', value: 'option2' }, + { label: 'option 3', value: 'option3', disabled: true }, + { label: 'option 4', value: 'option4' }, + { label: 'option 5', value: 'option5', selected: true }, + ])} > - - - - - - - diff --git a/packages/ui-library/src/components/select/index.css.ts b/packages/ui-library/src/components/select/index.css.ts index 7790bd2aa..979284bc4 100644 --- a/packages/ui-library/src/components/select/index.css.ts +++ b/packages/ui-library/src/components/select/index.css.ts @@ -22,10 +22,6 @@ export const staticStyles = css` display: flex; } - slot { - display: none; - } - .blr-select-inner-container { flex-grow: 1; flex-shrink: 1; diff --git a/packages/ui-library/src/components/select/index.stories.ts b/packages/ui-library/src/components/select/index.stories.ts index 64a134151..5072fb324 100644 --- a/packages/ui-library/src/components/select/index.stories.ts +++ b/packages/ui-library/src/components/select/index.stories.ts @@ -37,6 +37,14 @@ const defaultParams: BlrSelectType = { arialabel: 'Select', selectId: 'selectId', name: 'select', + options: [ + { label: '--Please choose an option--', value: '' }, + { label: 'option 1', value: 'option1' }, + { label: 'option 2', value: 'option2' }, + { label: 'option 3', value: 'option3', disabled: true }, + { label: 'option 4', value: 'option4' }, + { label: 'option 5', value: 'option5', selected: true }, + ], }; export default { @@ -232,17 +240,7 @@ Select presents users with a list of options from which they can make a single s }, }; -const optionsAsChildren = html` - - - - - - - -`; - -export const BlrSelect = (params: BlrSelectType) => BlrSelectRenderFunction(params, optionsAsChildren); +export const BlrSelect = (params: BlrSelectType) => BlrSelectRenderFunction(params); BlrSelect.storyName = 'Select'; BlrSelect.args = defaultParams; @@ -293,33 +291,24 @@ export const SizeVariant = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - sizeVariant: 'sm', - label: 'Select SM', - labelAppendix: '', - }, - optionsAsChildren - )} - ${BlrSelectRenderFunction( - { - ...defaultParams, - sizeVariant: 'md', - label: 'Select MD', - labelAppendix: '', - }, - optionsAsChildren - )} - ${BlrSelectRenderFunction( - { - ...defaultParams, - sizeVariant: 'lg', - label: 'Select LG', - labelAppendix: '', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + sizeVariant: 'sm', + label: 'Select SM', + labelAppendix: '', + })} + ${BlrSelectRenderFunction({ + ...defaultParams, + sizeVariant: 'md', + label: 'Select MD', + labelAppendix: '', + })} + ${BlrSelectRenderFunction({ + ...defaultParams, + sizeVariant: 'lg', + label: 'Select LG', + labelAppendix: '', + })}
`; }; @@ -338,15 +327,12 @@ export const Disabled = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - disabled: true, - label: 'Disabled', - labelAppendix: '', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + disabled: true, + label: 'Disabled', + labelAppendix: '', + })}
`; }; @@ -365,15 +351,12 @@ export const Required = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - required: true, - label: 'Required', - labelAppendix: '', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + required: true, + label: 'Required', + labelAppendix: '', + })}
`; }; @@ -389,16 +372,13 @@ export const HasError = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - hasError: true, - errorMessageIcon: undefined, - label: 'Error', - labelAppendix: '', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + hasError: true, + errorMessageIcon: undefined, + label: 'Error', + labelAppendix: '', + })}
`; }; @@ -415,22 +395,16 @@ export const FormLabel = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - label: 'With Label', - labelAppendix: '(with Appendix)', - }, - optionsAsChildren - )} - ${BlrSelectRenderFunction( - { - ...defaultParams, - label: ' ', - labelAppendix: ' ', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + label: 'With Label', + labelAppendix: '(with Appendix)', + })} + ${BlrSelectRenderFunction({ + ...defaultParams, + label: ' ', + labelAppendix: ' ', + })}
`; }; @@ -446,24 +420,18 @@ export const Icon = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - icon: 'blrArrowUp', - label: 'With Icon', - labelAppendix: ' ', - }, - optionsAsChildren - )} - ${BlrSelectRenderFunction( - { - ...defaultParams, - icon: undefined, - label: 'Default Icon', - labelAppendix: ' ', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + icon: 'blrArrowUp', + label: 'With Icon', + labelAppendix: ' ', + })} + ${BlrSelectRenderFunction({ + ...defaultParams, + icon: undefined, + label: 'Default Icon', + labelAppendix: ' ', + })}
`; }; @@ -477,28 +445,22 @@ export const FormCaptionGroup = () => { return html` ${sharedStyles}
- ${BlrSelectRenderFunction( - { - ...defaultParams, - hasHint: true, - label: 'Hint message', - labelAppendix: ' ', - }, - optionsAsChildren - )} - ${BlrSelectRenderFunction( - { - ...defaultParams, - icon: undefined, - label: 'Hint and error message', - labelAppendix: '', - hasHint: true, - hasError: true, - errorMessage: "OMG it's an error", - errorMessageIcon: 'blrErrorFilled', - }, - optionsAsChildren - )} + ${BlrSelectRenderFunction({ + ...defaultParams, + hasHint: true, + label: 'Hint message', + labelAppendix: ' ', + })} + ${BlrSelectRenderFunction({ + ...defaultParams, + icon: undefined, + label: 'Hint and error message', + labelAppendix: '', + hasHint: true, + hasError: true, + errorMessage: "OMG it's an error", + errorMessageIcon: 'blrErrorFilled', + })}
`; }; diff --git a/packages/ui-library/src/components/select/index.test.ts b/packages/ui-library/src/components/select/index.test.ts index e493905d5..ea8e423ef 100644 --- a/packages/ui-library/src/components/select/index.test.ts +++ b/packages/ui-library/src/components/select/index.test.ts @@ -1,10 +1,7 @@ import '@boiler/ui-library'; - import { BlrSelectRenderFunction } from './renderFunction.js'; - import { fixture, expect } from '@open-wc/testing'; import { querySelectorAllDeep, querySelectorDeep } from 'query-selector-shadow-dom'; -import { html } from 'lit-html'; import { BlrSelectType } from './index.js'; const sampleParams: BlrSelectType = { @@ -19,21 +16,18 @@ const sampleParams: BlrSelectType = { selectId: 'Peter', errorMessageIcon: 'blrErrorFilled', theme: 'Light', + options: [ + { label: 'option 1', value: 'option1' }, + { label: 'option 2', value: 'option2' }, + { label: 'option 3', value: 'option3', disabled: true }, + { label: 'option 4', value: 'option4' }, + { label: 'option 5', value: 'option5', selected: true }, + ], }; -const optionsAsChildren = html` - - - - - - - -`; - describe('blr-select', () => { it('is having a select containing the right className', async () => { - const element = await fixture(BlrSelectRenderFunction(sampleParams, optionsAsChildren)); + const element = await fixture(BlrSelectRenderFunction(sampleParams)); const select = querySelectorDeep('select', element.getRootNode() as HTMLElement); const className = select?.className; @@ -100,11 +94,4 @@ describe('blr-select', () => { expect(className).to.contain('sm'); }); - - it('is rendering options inside slot', async () => { - const element = await fixture(BlrSelectRenderFunction({ ...sampleParams, sizeVariant: 'sm' }, optionsAsChildren)); - const options = querySelectorAllDeep('.blr-select-option', element?.getRootNode() as HTMLElement); - const optionsLength = optionsAsChildren.strings[0].trim().split('').filter(Boolean).length; - expect(options).to.be.lengthOf(optionsLength); - }); }); diff --git a/packages/ui-library/src/components/select/index.ts b/packages/ui-library/src/components/select/index.ts index e89202048..bd2a1f30f 100644 --- a/packages/ui-library/src/components/select/index.ts +++ b/packages/ui-library/src/components/select/index.ts @@ -25,6 +25,13 @@ import { import { LitElementCustom, ElementInterface } from '../../utils/lit/element.js'; +type Options = { + label: string; + value: string; + disabled?: boolean; + selected?: boolean; +}; + export type BlrSelectEventHandlers = { blrSelectedValueChange?: (event: BlrSelectedValueChangeEvent) => void; blrFocus?: (event: BlrFocusEvent) => void; @@ -53,7 +60,6 @@ export class BlrSelect extends LitElementCustom { @property() accessor required: boolean | undefined; @property() accessor blrBlur: HTMLElement['blur'] | undefined; @property() accessor blrFocus: HTMLElement['focus'] | undefined; - @property() accessor hasError: boolean | undefined; @property() accessor errorMessage: string | undefined; @property() accessor hintMessage: string | undefined; @@ -61,13 +67,10 @@ export class BlrSelect extends LitElementCustom { @property() accessor errorMessageIcon: SizelessIconType | undefined; @property() accessor hasHint: boolean | undefined; @property() accessor icon: SizelessIconType | undefined = 'blrChevronDown'; - @property() accessor theme: ThemeType = 'Light'; - + @property() accessor options!: Options[] | JSON; @state() protected accessor isFocused = false; - protected _optionElements: Element[] | undefined; - protected handleFocus = (event: FocusEvent) => { if (!this.disabled) { this.isFocused = true; @@ -82,20 +85,6 @@ export class BlrSelect extends LitElementCustom { } }; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - protected firstUpdated(...args: Parameters): void { - if (!this._optionElements) { - this.handleSlotChange(); - } - } - - protected handleSlotChange() { - const slot = this.renderRoot?.querySelector('slot'); - - this._optionElements = slot?.assignedElements({ flatten: false }); - this.requestUpdate(); - } - protected handleChange(event: Event) { this.dispatchEvent( createBlrSelectedValueChangeEvent({ originalEvent: event, selectedValue: this._selectNode.value }) @@ -170,8 +159,6 @@ export class BlrSelect extends LitElementCustom { }); return html` - -
${this.hasLabel ? html` @@ -200,15 +187,15 @@ export class BlrSelect extends LitElementCustom { @focus=${this.handleFocus} @blur=${this.handleBlur} > - ${this._optionElements?.map((opt: Element) => { + ${(typeof this.options === 'string' ? JSON.parse(this.options) : this.options).map((opt: Options) => { return html` `; })} diff --git a/packages/ui-library/src/components/select/renderFunction.ts b/packages/ui-library/src/components/select/renderFunction.ts index 146ad7213..863294ed9 100644 --- a/packages/ui-library/src/components/select/renderFunction.ts +++ b/packages/ui-library/src/components/select/renderFunction.ts @@ -1,8 +1,7 @@ -import { TemplateResult } from 'lit-html'; import { BlrSelectType } from './index.js'; import { genericBlrComponentRenderer } from '../../utils/typesafe-generic-component-renderer.js'; export const TAG_NAME = 'blr-select'; -export const BlrSelectRenderFunction = (params: BlrSelectType, children?: TemplateResult<1>) => - genericBlrComponentRenderer(TAG_NAME, { ...params }, children); +export const BlrSelectRenderFunction = (params: BlrSelectType) => + genericBlrComponentRenderer(TAG_NAME, { ...params });