Skip to content

Commit

Permalink
fix(ld-select): maximum call stack size exceeded error
Browse files Browse the repository at this point in the history
  • Loading branch information
@borisdiakur
borisdiakur committed Jul 22, 2021
1 parent 4f0b128 commit cbe7d70
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 84 deletions.
8 changes: 8 additions & 0 deletions src/liquid/components/ld-option/ld-option-internal.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,19 @@

.ld-option-internal__check {
margin-right: var(--ld-sp-4);

.ld-select__popper--multiple & {
display: none;
}
}

.ld-option-internal__checkbox {
margin-left: var(--ld-sp-2);
margin-right: var(--ld-sp-6);

.ld-select__popper:not(.ld-select__popper--multiple) & {
display: none;
}
}

/* .ld-theme-ocean -> default */
Expand Down
70 changes: 31 additions & 39 deletions src/liquid/components/ld-option/ld-option-internal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,11 @@ export class LdOptionInternal {
*/
@Prop({ mutable: true, reflect: true }) selected = false

/**
* Prevents deselection of a selected option using a repeated mouse or keyboard interaction.
*/
@Prop() preventDeselection = false

/**
* Disables the option.
*/
@Prop() disabled = false

/**
* If true the option displays a checkbox, either checked or unchecked, instead of a simple check icon.
*/
@Prop() checkbox = false

/**
* Emitted on either selection or de-selection of the option.
*/
Expand All @@ -62,10 +52,15 @@ export class LdOptionInternal {
return
}

if (!this.selected || !this.preventDeselection) {
this.selected = !this.selected
if (
this.selected &&
this.el.closest('.ld-select__popper--prevent-deselection')
) {
return
}

this.selected = !this.selected

this.ldOptionSelect.emit(this.selected)
}

Expand Down Expand Up @@ -104,33 +99,30 @@ export class LdOptionInternal {
onClick={this.handleClick.bind(this)}
tabindex="-1"
>
{this.checkbox ? (
<ld-checkbox
role="presentation"
class="ld-option-internal__checkbox"
checked={this.selected}
disabled={this.disabled}
></ld-checkbox>
) : (
<svg
role={'presentation'}
class="ld-option-internal__check"
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<path
style={{ visibility: this.selected ? 'inherit' : 'hidden' }}
d="M15 7L8.40795 13L5 9.63964"
stroke="currentColor"
stroke-width="3"
stroke-linecap="round"
stroke-linejoin="round"
/>
</svg>
)}
<ld-checkbox
role="presentation"
class="ld-option-internal__checkbox"
checked={this.selected}
disabled={this.disabled}
></ld-checkbox>
<svg
role={'presentation'}
class="ld-option-internal__check"
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<path
style={{ visibility: this.selected ? 'inherit' : 'hidden' }}
d="M15 7L8.40795 13L5 9.63964"
stroke="currentColor"
stroke-width="3"
stroke-linecap="round"
stroke-linejoin="round"
/>
</svg>

<span
ref={(el) => (this.optionLabelRef = el as HTMLElement)}
Expand Down
10 changes: 0 additions & 10 deletions src/liquid/components/ld-option/ld-option.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,11 @@ export class LdOption {
*/
@Prop() selected = false

/**
* Prevents deselection of a selected option using a repeated mouse or keyboard interaction.
*/
@Prop() preventDeselection = false

/**
* Disables the option.
*/
@Prop() disabled = false

/**
* If true the option displays a checkbox, either checked or unchecked, instead of a simple check icon.
*/
@Prop() checkbox = false

componentWillLoad() {
applyPropAliases.apply(this)
}
Expand Down
16 changes: 7 additions & 9 deletions src/liquid/components/ld-option/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@

## Properties

| Property | Attribute | Description | Type | Default |
| -------------------- | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | ----------- |
| `checkbox` | `checkbox` | If true the option displays a checkbox, either checked or unchecked, instead of a simple check icon. | `boolean` | `false` |
| `disabled` | `disabled` | Disables the option. | `boolean` | `false` |
| `key` | `key` | for tracking the node's identity when working with lists | `string \| number` | `undefined` |
| `preventDeselection` | `prevent-deselection` | Prevents deselection of a selected option using a repeated mouse or keyboard interaction. | `boolean` | `false` |
| `ref` | `ref` | reference to component | `any` | `undefined` |
| `selected` | `selected` | If present, this boolean attribute indicates that the option is selected. | `boolean` | `false` |
| `value` | `value` | The content of this attribute represents the value to be submitted with the form, should this option be selected. If this attribute is omitted, the value is taken from the text content of the option element. | `string` | `undefined` |
| Property | Attribute | Description | Type | Default |
| ---------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | ----------- |
| `disabled` | `disabled` | Disables the option. | `boolean` | `false` |
| `key` | `key` | for tracking the node's identity when working with lists | `string \| number` | `undefined` |
| `ref` | `ref` | reference to component | `any` | `undefined` |
| `selected` | `selected` | If present, this boolean attribute indicates that the option is selected. | `boolean` | `false` |
| `value` | `value` | The content of this attribute represents the value to be submitted with the form, should this option be selected. If this attribute is omitted, the value is taken from the text content of the option element. | `string` | `undefined` |


----------------------------------------------
Expand Down
8 changes: 4 additions & 4 deletions src/liquid/components/ld-select/ld-select.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,6 @@ export class LdSelect {
throw new TypeError(
`ld-select accepts only ld-option elements as children, but found a "${tag}" element.`
)
} else if (this.multiple) {
child.setAttribute('checkbox', 'true')
} else if (this.preventDeselection) {
child.setAttribute('prevent-deselection', 'true')
}
})
}
Expand Down Expand Up @@ -859,10 +855,14 @@ export class LdSelect {
let popperCl = 'ld-select__popper'
if (this.invalid) popperCl += ' ld-select__popper--invalid'
if (detached) popperCl += ' ld-select__popper--detached'
if (this.multiple) popperCl += ' ld-select__popper--multiple'
if (this.expanded) popperCl += ' ld-select__popper--expanded'
if (this.size) popperCl += ` ld-select__popper--${this.size}`
if (this.themeCl) popperCl += ` ${this.themeCl}`
if (this.popperClass) popperCl += ` ${this.popperClass}`
if (this.preventDeselection) {
popperCl += ' ld-select__popper--prevent-deselection'
}

const triggerText = this.multiple
? this.placeholder
Expand Down
24 changes: 24 additions & 0 deletions src/liquid/components/ld-select/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,30 @@ The feature set of the `ld-select` Web Component differs from its CSS Component
</div>
{% endexample %}

#### Prevent deselection

You can prevent a state with no options selected after initial selection in single select mode.

{% example %}
<ld-select placeholder="Pick a fruit" name="fruit" prevent-deselection>
<ld-option value="apple">Apple</ld-option>
<ld-option value="banana">Banana</ld-option>
<ld-option value="strawberry">Strawberry</ld-option>
<ld-option value="watermelon" disabled>Watermelon</ld-option>
<ld-option value="honeymelon">Honeymelon</ld-option>
<ld-option value="rasberry">Rasberry</ld-option>
<ld-option value="cherry">Cherry</ld-option>
<ld-option value="blueberry">Blueberry</ld-option>
<ld-option value="peach">Peach</ld-option>
<ld-option value="grape">Grape</ld-option>
<ld-option value="fuyu persimmon">Fuyu Persimmon</ld-option>
<ld-option value="monstera deliciosa">Monstera Deliciosa</ld-option>
<ld-option value="pear">Pear</ld-option>
<ld-option value="pineapple">Pineapple</ld-option>
<ld-option value="plum">Plum</ld-option>
</ld-select>
{% endexample %}

### Multiple select mode

{% example %}
Expand Down
38 changes: 16 additions & 22 deletions src/liquid/components/ld-select/test/ld-select.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,13 @@ describe('ld-select', () => {
expect(spyBlur).toHaveBeenCalledTimes(1)
})

it('uses checkboxes on options in multiple mode', async () => {
it('passes down prop selected option prop to internal options', async () => {
const page = await newSpecPage({
components: [LdSelect],
html: `
<ld-select placeholder="Pick a fruit" name="fruit" multiple>
<ld-select placeholder="Pick a fruit" name="fruit">
<ld-option value="apple">Apple</ld-option>
<ld-option value="banana">Banana</ld-option>
<ld-option value="banana" selected='true'>Banana</ld-option>
</ld-select>
`,
})
Expand All @@ -189,17 +189,17 @@ describe('ld-select', () => {
const internalOptions = popper.querySelectorAll('ld-option-internal')

expect(internalOptions.length).toEqual(2)
expect(internalOptions[0].getAttribute('checkbox')).toEqual('true')
expect(internalOptions[1].getAttribute('checkbox')).toEqual('true')
expect(internalOptions[0].getAttribute('selected')).toEqual(null)
expect(internalOptions[1].getAttribute('selected')).toEqual('true')
})

it('passes down prop selected option prop to internal options', async () => {
it('sets prevent deselection class on popper element', async () => {
const page = await newSpecPage({
components: [LdSelect],
html: `
<ld-select placeholder="Pick a fruit" name="fruit">
<ld-select placeholder="Pick a fruit" name="fruit" prevent-deselection>
<ld-option value="apple">Apple</ld-option>
<ld-option value="banana" selected='true'>Banana</ld-option>
<ld-option value="banana">Banana</ld-option>
</ld-select>
`,
})
Expand All @@ -208,18 +208,17 @@ describe('ld-select', () => {

const body = page.body
const popper = body.querySelector('.ld-select__popper')
const internalOptions = popper.querySelectorAll('ld-option-internal')

expect(internalOptions.length).toEqual(2)
expect(internalOptions[0].getAttribute('selected')).toEqual(null)
expect(internalOptions[1].getAttribute('selected')).toEqual('true')
expect(
popper.classList.contains('ld-select__popper--prevent-deselection')
).toBeTruthy()
})

it('passes down prop preventDeselection to internal options', async () => {
it('sets multiple class on popper element', async () => {
const page = await newSpecPage({
components: [LdSelect],
html: `
<ld-select placeholder="Pick a fruit" name="fruit" prevent-deselection>
<ld-select placeholder="Pick some fruits" name="fruits" multiple>
<ld-option value="apple">Apple</ld-option>
<ld-option value="banana">Banana</ld-option>
</ld-select>
Expand All @@ -230,14 +229,9 @@ describe('ld-select', () => {

const body = page.body
const popper = body.querySelector('.ld-select__popper')
const internalOptions = popper.querySelectorAll('ld-option-internal')

expect(internalOptions.length).toEqual(2)
expect(internalOptions[0].getAttribute('prevent-deselection')).toEqual(
'true'
)
expect(internalOptions[1].getAttribute('prevent-deselection')).toEqual(
'true'
)
expect(
popper.classList.contains('ld-select__popper--multiple')
).toBeTruthy()
})
})

0 comments on commit cbe7d70

Please sign in to comment.