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

[range-slider] #1385

Closed
cuberoot opened this issue Apr 16, 2021 · 14 comments · Fixed by #1416
Closed

[range-slider] #1385

cuberoot opened this issue Apr 16, 2021 · 14 comments · Fixed by #1416

Comments

@cuberoot
Copy link
Collaborator

cuberoot commented Apr 16, 2021

Multi-Handle Slider Proposal

This began as a proposal for an implementation of the range slider as documented in Spectrum Contributions and as implemented in spectrum-css.

The discussion (see below) has led us to a more general extension to sliders to allow multiple handles. This is the updated reworked proposal.

The basic gist of the proposal is to allow the user to specify the handles explicitly using an sp-slider-handle element as per this example:

<sp-slider min=0 max=10>
  <sp-slider-handle name="x" value="0" min="0" max="next"></sp-slider-handle>
  <sp-slider-handle name="y" value="1" min="0" max="8"></sp-slider-handle>
  <sp-slider-handle name="z" value="3" min="previous" max="10"></sp-slider-handle> 
</sp-slider>

Node that in the absence of specified name, the handles will be named handle1, handle2, etc.

Backwards compatibility and the simple case

In the case where only one handle is needed, we will read the value, min and max properties off of the sp-slider itself. It will act as the sp-slider-handle.

<sp-slider value="0" min="0" max="10"></sp-slider>

Variants

It is likely that we will only support the default variant when there is more than one handle.

Handle clamping

In most cases, but not all, we will want to enforce the ordering of the handles. When ordering is enforced, handles will not be able to move past each other. The API will take the form of min and max properties on the handles.

attr allowed values
min 'previous' | number | undefined
max 'next' | number | undefined

The values previous and next will prevent a handle from moving past the adjacent handle in that direction.

Label formatting

Currently we support a getAriaValueText property which is a function that does the formatting. Our solution need to take into account #1159. See comments below. This section needs to be fleshed out further.

Each sp-slider-handle and the sp-slider will have a property like this:

    @property({ type: Object, attribute: 'format-options' })
    public formatOptions: Intl.NumberFormatOptions = {};

The getAriaValueText property will be modified to take a function of type (values: Map<string, string>) => string. In the case where there is only one slider, the map will just contain the the pair 'value' => <value of slider. The string values will be the individual slider values by name as formatted according to their formatOptions. If an sp-slider-handle does not have a value for formatOptions than the formatOptions from the host sp-slider will be used.

Question: When there is only one implicit sp-slider-handle, should we just pass the value as a string instead of in a Map? Is a Map the right structure. I chose it because it is both random access and ordered.

Events

The events should look the same as for the standard slider with change and input events. The change and input events will be per-handle and will come from the sp-slider-handle element. Where there are no explicitly declared sp-slider-handle elements (there is only one default handle), the events will come from the sp-slider itself.

values property

The value property that will return an array of objects with the name and value for each handle such as:

[
    { name: 'x', value: 0 },
    { name: 'x, value: 1 },
    { name: 'z, value: 3 }
]

Interactions

From playing with other implementations, it appears that the most recently dragged handle is always on top. The keyboard should interact with the currently selected handle. And you should be able to tab between handles.

Breaking changes

  • You will not longer be able to set the label via slotted text as that gets confusing when there could be sp-slider-handle children.
  • getAriaValueText function property will now be passed a Map<string, string> with the handle names and values.
@Westbrook
Copy link
Contributor

Westbrook commented Apr 16, 2021

This sounds great. Thanks for bringing this information together! Do you think that you or your team will be able to contribute something like this?

Label Formatting

We should make sure to take into account #1159 when thinking about the label formatting approach. Having the general API be as close to the standard sp-slider as possible will be super useful, and while we've not activated advance capabilities like this in sp-color-slider, etc. they'll all likely benefit from shared decisions.

There's also the possibility that we will need specific formatting for the value of each of the handles, as they will be their own tab stops and their own <input type="range"> under the covers (which further supports your notes in the Interactions section above):

<div class="spectrum-Slider spectrum-Slider--range" role="group" aria-labelledby="spectrum-Slider-label-4">
  <div class="spectrum-Slider-labelContainer" role="presentation">
    <label class="spectrum-Slider-label" id="spectrum-Slider-label-4" for="spectrum-Slider-input-4-0">Slider Label</label>
    <div class="spectrum-Slider-value" role="textbox" aria-readonly="true" aria-labelledby="spectrum-Slider-label-4">14 - 48</div>
  </div>
  <div class="spectrum-Slider-controls" role="presentation">
    <div class="spectrum-Slider-track" style="width: 20%;"></div>
    <div class="spectrum-Slider-handle" style="left: 20%;" role="presentation">
      <input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" aria-label="min" id="spectrum-Slider-input-4-0" aria-labelledby="spectrum-Slider-label-4 spectrum-Slider-input-4-0">
    </div>
    <div class="spectrum-Slider-track" style="left: 20%; right: 40%;"></div>
    <div class="spectrum-Slider-handle" style="left: 60%;" role="presentation">
      <input type="range" class="spectrum-Slider-input" value="14" step="2" min="10" max="20" aria-label="max" id="spectrum-Slider-input-4-1" aria-labelledby="spectrum-Slider-label-4 spectrum-Slider-input-4-1">
    </div>
    <div class="spectrum-Slider-track" style="width: 40%;"></div>
  </div>
</div>

In reference to the above, a having curtom label formatting for the start/end values would support an implementor choosing to inform the screen reader to read something like Slider Label 14 - 48, Starting Value 14 and Slider Label 14 - 48, Ending Value 48 if they so chose.

Value

For the value updates, I wonder what sort of use case this would be leveraged in and if they would find any benefit from a value = [start, end] structure. I like you suggestion to have them separate, but I'm a little remiss to completely remove value as an accessor as it's the "expected" interface for this sort of data in a native form element.

Min/Max/Defaults

It would seem that min/max can roughly be managed the same as sp-slider implementation, but what you you think we should de with the defaults? sp-slider defaults to 10, and I have no clue where that came from. We recently changed the default max to more closely align with a native <input type="range"> but I've not got many thoughts on what the default for the stat and end values should be in this context.

@Westbrook
Copy link
Contributor

One other thing to think about in regards to this API shape: value-start="10" value-end="80", if we were to want to say add a third handle, do you have any thoughts on how we'd be able to do that? Like, what sort of attributes API would be needed to support more stops here?

@cuberoot
Copy link
Collaborator Author

So, I am working on this implementation right now. So, yes we can contribute. We need this yesterday 😄

Here's what I am playing with to support multiple stops in a way that allows expansion and customization. As I mentioned, the first thing I will do with this is customize it to add a third stop for a particular use case.

I am am proposing splitting Slider.ts into SliderBase.ts and Slider.ts. I am thinking of adding abstract methods to the base class like so:

    protected abstract valueForHandle(handle: HTMLElement): number;
    protected abstract setValueForHandle(name: string, value: number): void;

That would allow the implementation to get/set the value for a handle. Those methods would interact with the value attributes such as --value-end. By overloading those methods, we could teach the slider about new values. In the React Spectrum implementation, they have a single structured attribute, but that doesn't work as well with web components. So, I had presumed that we would keep an attribute per slider value with a little internal glue for each implementation.

Internally, I am looking at the concept of a handle stack. Handles should stack in z based on how recently they have been interacted with. So, a handle that you have just dragged should be on top of other handles. Here's what the implementation I am playing with looks like:

export type Handle = {
    handle: HTMLDivElement;
    input: HTMLInputElement;
};

export type HandleOrEmpty = {
    handle?: HTMLDivElement;
    input?: HTMLInputElement;
}

export class HandleStack<HandleNames extends string> {
    private handles: Handle[] = [];

    constructor(root: ShadowRoot) {
        const inputs = root.querySelectorAll('.handle > input[type="range"]');
        this.handles = Array.from(inputs).map(input => {
            const inputElem = input as HTMLInputElement;
            const handleElem = inputElem.parentElement as HTMLDivElement;
            return {
                handle: handleElem,
                input: inputElem,
            };

        });
    }

    public get activeHandle(): Handle {
        return this.handles[this.handles.length - 1];
    }

    public get activeHandleName(): string {
        const { handle } = this.handles[this.handles.length - 1];
        return handle.id;
    }

    public activateHandle(handleElement: HTMLDivElement): void {
        const index = this.handles.findIndex(item => item.handle === handleElement);
        if (index > 0) {
            const handle = this.handles[index];
            this.handles.splice(index, 1);
            this.handles.push(handle);
        }
    }

    public handleByName(name: HandleNames): HandleOrEmpty {
        return this.handles.find(item => item.handle.id === name) as Handle ?? {};
    }

    public indexOfHandle(name: HandleNames): number {
        for (let index = 0; index < this.handles.length; index++) {
            const item = this.handles[index];
            if (item.handle.id === name) {
                return index;
            }
        }
        return 0;
    }
}

The handle stack gets the handle divs and their inputs by doing a selector query, so it will pick up new handles added by subclassed implementations.

@Westbrook
Copy link
Contributor

Westbrook commented Apr 21, 2021

I like the structure here for managing handles. Could you help me to better understand how you'd use it in over the life of the element? Particularly, how you'd see it specifically powering the API for 2 or more handles at the SliederBase.ts level?

For instance what would the DOM/attribute interface be as you scale up the number of handles based off of the named access?

  • Do you expect a meta code style access to attributes? <sp-slider handle-only="value">, <sp-range-slider handle-start="value" handle-end="value">, <sp-double-range-slider handle-first="value" handle-second="value" handle-third="value">.
  • I'm not fully opposed to a complex attribute setter: <sp-range-slider value='[{"name": "name1", "value": 0},{"name": "name2", "value": 10}]'> or similar. When paired appropriately with a property setter most uses within an application are not negatively affected by the DX this surfaces.
  • With the above it's easy to see expanding into a declarative API pattern that might look like:
    <sp-slider>
      <sp-slider-handle name="x" value="0"></sp-slider-handle>
      <sp-slider-handle name="y" value="1"></sp-slider-handle>
      <sp-slider-handle name="z" value="3"></sp-slider-handle> 
    </sp-slider>
    A similar approach is on tap for the <sp-combobox> API and could benefit from shared tooling like: https://webcomponents.dev/edit/F4jBbQpeMSujg9FtRrtZ/src/index.stories.js

I'd much prefer constructing the stack from data, rather than reconstructing the stack from DOM. It may be that we leverage the stack to build the DOM? Something along the lines of the following pseudo code:

class RangeSlider extends SliderBase {
   private handleStack = new HandleStack(this, 2);

  render() {
      return html`
       // ...
      ${this.handleStack.render(listeners, whatever)}
      // ...
     `;
  }
}

export class HandleStack<HandleNames extends string> {
   render() {
    return this.handles.map(this.renderHandle);
  }

  renderHandle(handle, index) {
    return html`
        <div class="handle" role="presentation" style="left: ${this.asPercent(handle.value)}; z-index: ${index + 1};">
                <input type="range" id="input" value="${handle.value}" step="etc" min="etc" max="etc" aria-valuetext="${this.asPercent(handle.value)}">
            </div>
    `;
  }
}

@cuberoot
Copy link
Collaborator Author

Hmm, I like the idea of constructing the stack and rendering the handles from that... Thanks for that input..

For the declarative API idea. That also sounds interesting. How would we specify the interactions between handles? In some cases, the handles must be strictly ordered and in some they can slide past each other. I can think of examples of both cases...

<sp-slider enforce-handle-order>
  <sp-slider-handle name="x" value="0"></sp-slider-handle>
  <sp-slider-handle name="y" value="1"></sp-slider-handle>
  <sp-slider-handle name="z" value="3"></sp-slider-handle> 
</sp-slider>

Something like this? Only with a better name than enforce-handle-order?

@Westbrook
Copy link
Contributor

Definitely could work. Alternative API might be to include min/max on each handle, types to 'previous' | 'next' | number or similar?

<sp-slider min=0 max=10>
  <sp-slider-handle name="x" value="0" min="0" max="next"></sp-slider-handle>
  <sp-slider-handle name="y" value="1" min="0" max="8"></sp-slider-handle>
  <sp-slider-handle name="z" value="3" min="previous" max="10"></sp-slider-handle> 
</sp-slider>

One case to think about is whether you support one handle moving another. For instance, if you added something like a concedes attribute to the final handle above and then moved the middle handle to 8, it would also move the third handle to 8...

What's the use case on the order that can change? I definitely could see your API supporting the two options, but I'd wonder are we actually delivering a quality UX for user in that context. Seems a bit confusing, but maybe the context it's relative to give it light there?

@cuberoot
Copy link
Collaborator Author

cuberoot commented Apr 22, 2021

I do not currently have a use case for one slider moving another, although I can imagine there might be a use case. So, I would like to leave that as work for whoever hits that case, but we shouldn't design an API that could not extend to support it.

One oddity I do have is a three-slider example where the middle slider maintains proportional spacing as you move the outer sliders. I presume I could custom code that with events.

@cuberoot
Copy link
Collaborator Author

One other thing I need is a way to specify the background colour of the handles. I know that is outside of Spectrum proper, but it's necessary in my current UI...

@Westbrook
Copy link
Contributor

I really like how this is coming together, please let me know if there is anything I can do to support!

Variants

In not supporting variants (at least for now), would the idea be that having more than one <sp-slider-handle> child would force the root element to the default variant? Or would we manage this state (which seems like it would amount to an error otherwise) some other way?

Label formatting

I like keeping the changes here as slim as possible, but I have started to work with a formatOptions property in a Number Field prototype and it seems quite powerful. Not sure we can fully solve labeling without only it, so it may be that this and the getAriaValueText work together?

Values property

I think a property like this makes sense. Should the sp-slider element's value property simply take this shape when multiple handles are present?

Also, would a property in this way better serve a user typed like:

type SliderValues = [
    { name: 'Handle 1', value: 0 },
    { name: 'Handle 2', value: 1 },
    { name: 'Handle 3', value: 2 }
]

This way you can guarantee later passes over the handles appear in order?

@cuberoot
Copy link
Collaborator Author

Variants

I had imagined that the root element (the sp-slider) would force itself into the default variant and issue a warning to the console if an unsupported variant was specified. It may be that some or all of the variants are easy to support with the segmented track. I will know that soon.

Label formatting

I like the combination of the formatOptions and getAriaValueText. Maybe we pass the formatted values into getAriaValueText as an argument?

Values property

I can try to make the value property take a different shape when there are multiple handles. The question is whether I can make Lit and Typescript happy with that. I like the shape that you propose. The other option is to return a Map which is actually an ordered container.

@adixon-adobe
Copy link
Collaborator

In looking at this and thinking through it, I'm feeling like we would be better served by having a separate sp-range-slider component to support this. Things like the value attribute & getAriaValueText changing format, and the label slot change are indicators to me that it's worth splitting this out.

I think it's also worth thinking about how editable value labels & the sort of "target values" we use in Lightroom tutorials might work here. It think there are some UI concepts/improvements that might work well with the single-slider case but don't make sense for the range slider case (or might take a different form).
target-value

@Westbrook
Copy link
Contributor

I bet there'a a nice way to leverage the <sp-slider-handle> interface here to surface an internally realized version of this helper/education UI. 🤔

@adixon-adobe Does this get outlined in Spectrum specifically, or are is the education later something your team is adding with less central input?

@adixon-adobe
Copy link
Collaborator

It's eluded to here https://spectrum-contributions.corp.adobe.com/page/tutorial-coach-mark-beta/ but the impact to slider isn't specifically called out.

cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 27, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 31, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 31, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue May 31, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 1, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 1, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 1, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 2, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 2, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 3, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 3, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 3, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 3, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 10, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 10, 2021
Implement multi-handle slider support as discussed in #1385
cuberoot pushed a commit that referenced this issue Jun 10, 2021
Implement multi-handle slider support as discussed in #1385
Westbrook pushed a commit that referenced this issue Jun 11, 2021
Implement multi-handle slider support as discussed in #1385
Westbrook pushed a commit that referenced this issue Jun 11, 2021
Implement multi-handle slider support as discussed in #1385
Westbrook pushed a commit that referenced this issue Jun 11, 2021
Implement multi-handle slider support as discussed in #1385
@Westbrook
Copy link
Contributor

This shipped 🎊 ...a while back. Thanks again @cuberoot!!

For anyone who participated in the conversation above, there may be some nice nuggets for extension in the notes above, so feel free to open new issues as you see fit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants