[Bug]: sp-slider only adds one sp-slider-handle when multiple are defined #2035

peterjanes · 2022-01-07T15:37:32Z

Code of conduct

I agree to follow this project's code of conduct.

Impacted component(s)

sp-slider, sp-slider-handle

Expected behavior

Adding an sp-slider with multiple handles from an HTML template should add all of the handles.

Actual behavior

I haven't nailed down the exact conditions, but sometimes when adding sp-sliders dynamically from an HTML template only one instance of sp-slider-handle will be added because the handle map key is undefined. (I finally tracked down the behaviour when I started getting "First slider handle cannot have attribute min='previous'" after adding max="next" to the first sp-slider-handle and min="previous" to the second.)

Screenshots

No response

What browsers are you seeing the problem in?

Firefox, Chrome

How can we reproduce this issue?

I haven't been able to come up with a reduced test case, but the issue is 100% reproducible in my (currently non-public) app and seems to be due to some instances of a field not being renamed. (handle.name is used instead of handle.handleName in HandleController.ts.) I'll create an associated pull request for review.

Sample code that illustrates the problem

No response

Logs taken while reproducing problem

No response

The text was updated successfully, but these errors were encountered:

Westbrook · 2022-01-07T16:21:45Z

@peterjanes thanks for sharing this issue!

Before we can fully review changes in support of what you're experiencing, we will need a reproduction case so that we can compare your findings with ours and ensure that all users of this element are being appropriately served. I'd suggest you start from something like https://webcomponents.dev/edit/collection/fO75441E1Q5ZlI0e9pgq/U7LQv7LsAVBwJayJXG3B/src/index.ts and see if you can isolate the issue you are experiencing.

peterjanes · 2022-01-07T16:28:09Z

Yeah, I've been working on a reproduce case for a couple of days but can't get it to fail. Figured I'd submit the PR that fixes it (or appears to) as it seems like a pretty obvious update; handle.name is only set when handle.handleName isn't set (and only used in a log message), and the undefined handle.handleName is immediately used as the map key (so only the last handle added will ever be present).

peterjanes · 2022-01-07T17:03:16Z

Here's my current non-failing reproduce case, which mirrors what the live code does:

<!DOCTYPE html>
<script src="https://jspm.dev/@spectrum-web-components/bundle/elements.js" type="module"></script>

<div>
<sp-theme scale="medium" color="light">
</sp-theme>
</div>

<template id="rangeTemplate">
  <sp-slider variant="range" step="1">
    <sp-slider-handle slot="handle" name="min" label="Minimum" max="next"></sp-slider-handle>
    <sp-slider-handle slot="handle" name="max" label="Maximum" min="previous"></sp-slider-handle>
  </sp-slider>
</template>

<script module>
async function foo() {
  const res = await fetch('.');
  const rangeGroups = new Map();
  rangeGroups.set('group1', [{id: 'range1.1', min: 10, max: 90}, {id: 'range1.2', min: 15, max: 85}]);
  rangeGroups.set('group2', [{id: 'range2.1', min: 20, max: 80}, {id: 'range2.2', min: 25, max: 75}]);

  const rangesList = document.createElement('div');
  const rangeTemplate = document.querySelector('#rangeTemplate');

  rangeGroups.forEach((rangeGroup, key) => {
    const rangeList = document.createElement('div');
    rangeGroup.forEach((range) => {
      const rangeElement = rangeTemplate.content.cloneNode(true);
      const slider = rangeElement.querySelector('sp-slider');
      slider.setAttribute('id', range.id);
      slider.setAttribute('label', range.id);
      slider.setAttribute('min', String(range.min));
      slider.setAttribute('max', String(range.max));
      slider.querySelector('[name=min]').setAttribute('value', String(range.min));
      slider.querySelector('[name=max]').setAttribute('value', String(range.max));
      rangeList.appendChild(rangeElement);
    });
    rangesList.appendChild(rangeList);
  });
  document.querySelector('sp-theme').appendChild(rangesList);
  return 'done';
}
foo().then(console.log);
</script>

Westbrook · 2022-01-07T17:08:41Z

I assume you're not using <script src="https://jspm.dev/@spectrum-web-components/bundle/elements.js" type="module"></script> in your production code. Is there anything about how you're importing the elements that you can share, along with which versions you're leveraging?

peterjanes · 2022-01-07T17:10:52Z

In the live app I've reduced that whole thing to

function createElement(html) {
  const temp = document.createElement('template');
  temp.innerHTML = html.trim();
  return temp.content.firstChild;
}

async function foo() {
  await fetch('.');
  document.body.appendChild(
      createElement(`<div>
  <sp-slider variant="range" step="1" id="price" name="price" label="Max Price" min="35425" max="86610">
    <sp-slider-handle slot="handle" name="min" label="Minimum" max="next" value="35425"></sp-slider-handle>
    <sp-slider-handle slot="handle" name="max" label="Maximum" min="previous" value="86610"></sp-slider-handle>
  </sp-slider>
</div>`);
}

foo();

and it still fails.

The app loads the dependencies with

import '@spectrum-web-components/theme/theme-light';
import '@spectrum-web-components/theme/scale-medium';
import '@spectrum-web-components/theme/sp-theme';
import '@spectrum-web-components/button/sp-button';
import '@spectrum-web-components/slider/sp-slider';
import '@spectrum-web-components/slider/sp-slider-handle';

from

    "@spectrum-web-components/button": "^0.16.1",
    "@spectrum-web-components/slider": "^0.12.1",
    "@spectrum-web-components/theme": "^0.9.1",

Westbrook · 2022-01-07T17:12:29Z

How is JS loaded into the app? For instance this (working demo from above) uses <script type="module"> which works different that a <script>, a dynamic import, etc.

peterjanes · 2022-01-07T17:15:32Z

<script type="module" src="script.js"></script>

peterjanes · 2022-01-07T17:16:42Z

I'm going to try again to strip down the app to something I can share instead of trying to build up a test case. Will let you know.

peterjanes · 2022-01-07T17:48:09Z

https://home.peterjanes.ca/~peterj/2022/bug-2035/ has a fairly minimal failing test case, with no recompilation, subsetting, etc. of the Spectrum components.

hunterloftis · 2022-01-10T16:33:47Z

Thanks for the minimal example @peterjanes - I've used it to add a test reproducing the issue so that we can figure out a root cause.

peterjanes added bug Something isn't working triage An issue needing triage labels Jan 7, 2022

Westbrook assigned hunterloftis Jan 7, 2022

Westbrook added API Component: Range Slider Needs repro and removed triage An issue needing triage labels Jan 7, 2022

peterjanes mentioned this issue Jan 7, 2022

Use handleName field consistently in HandleController #2036

Closed

10 tasks

hunterloftis mentioned this issue Jan 10, 2022

fix(slider): ensure that handles are upgraded before extracting model #2040

Merged

10 tasks

peterjanes mentioned this issue Jan 11, 2022

[Bug]: Only the rightmost sp-slider-handle child is active in an sp-slider[variant=range] #2032

Closed

1 task

hunterloftis closed this as completed in #2040 Jan 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: sp-slider only adds one sp-slider-handle when multiple are defined #2035

[Bug]: sp-slider only adds one sp-slider-handle when multiple are defined #2035

peterjanes commented Jan 7, 2022 •

edited

Westbrook commented Jan 7, 2022

peterjanes commented Jan 7, 2022 •

edited

peterjanes commented Jan 7, 2022

Westbrook commented Jan 7, 2022

peterjanes commented Jan 7, 2022

Westbrook commented Jan 7, 2022

peterjanes commented Jan 7, 2022

peterjanes commented Jan 7, 2022

peterjanes commented Jan 7, 2022

hunterloftis commented Jan 10, 2022

[Bug]: sp-slider only adds one sp-slider-handle when multiple are defined #2035

[Bug]: sp-slider only adds one sp-slider-handle when multiple are defined #2035

Comments

peterjanes commented Jan 7, 2022 • edited

Code of conduct

Impacted component(s)

Expected behavior

Actual behavior

Screenshots

What browsers are you seeing the problem in?

How can we reproduce this issue?

Sample code that illustrates the problem

Logs taken while reproducing problem

Westbrook commented Jan 7, 2022

peterjanes commented Jan 7, 2022 • edited

peterjanes commented Jan 7, 2022

Westbrook commented Jan 7, 2022

peterjanes commented Jan 7, 2022

Westbrook commented Jan 7, 2022

peterjanes commented Jan 7, 2022

peterjanes commented Jan 7, 2022

peterjanes commented Jan 7, 2022

hunterloftis commented Jan 10, 2022

peterjanes commented Jan 7, 2022 •

edited

peterjanes commented Jan 7, 2022 •

edited