adobe / spectrum-web-components

Spectrum Web Components
https://opensource.adobe.com/spectrum-web-components/
Apache License 2.0
1.21k stars 192 forks source link

[Bug]: Un-necessary #label-container in the DOM for label-visibility="none" #3836

Open Rajdeepc opened 7 months ago

Rajdeepc commented 7 months ago

Code of conduct

Impacted component(s)

Slider

Expected behavior

When user sets label-visibility=none on sp-slider the element with id label-container should not be in the DOM. This is causing the sp-slider to move up with a custom label outside the shadowroot of slider.

Actual behavior

If label-visibility=none then we should not render <div id="label-container"></div> . This will fix the position of the slider

Screenshots

UI issue on slider. Slider track is moving up.

Screenshot 2023-11-30 at 1 05 20 PM

What browsers are you seeing the problem in?

No response

How can we reproduce this issue?

  1. Go to https://opensource.adobe.com/spectrum-web-components/storybook/index.html?path=/story/slider--no-visible-labels
  2. Open dev tools
  3. See <div id="label-container"></div> is still there which is causing the slider to move up.

Sample code that illustrates the problem

private renderLabel(): TemplateResult {
        const textLabelVisible =
            this.labelVisibility === 'none' || this.labelVisibility === 'value';
        const valueLabelVisible =
            this.labelVisibility === 'none' || this.labelVisibility === 'text';
        return html`
            <div id="label-container">
                <sp-field-label
                    class=${classMap({
                        'visually-hidden': textLabelVisible,
                    })}
                    ?disabled=${this.disabled}
                    id="label"
                    for=${this.editable
                        ? 'number-field'
                        : this.handleController.activeHandleInputId}
                    @click=${this.handleLabelClick}
                    size=${this.size}
                >
                    ${this.slotHasContent ? nothing : this.label}
                    <slot>${this.label}</slot>
                </sp-field-label>
                <sp-field-label
                    class=${classMap({
                        'visually-hidden': valueLabelVisible,
                    })}
                    ?disabled=${this.disabled}
                    for=${this.editable
                        ? 'number-field'
                        : this.handleController.activeHandleInputId}
                    size=${this.size}
                >
                    <output id="value" aria-live="off" for="input">
                        ${this.ariaValueText}
                    </output>
                </sp-field-label>
            </div>
        `;
    }

Logs taken while reproducing problem

No response

Westbrook commented 7 months ago

For reference: this element, and its children continue to play a role in accessibly delivering the inputs within this pattern as long as slot content or this.label is provided for the first and a value is managed for the second.

Westbrook commented 6 months ago

When you compare the default and custom delivery of a label slider, a la https://studio.webcomponents.dev/edit/U7LQv7LsAVBwJayJXG3B/src/index.ts?branch=custom-label%40wfVpXdsTYhf9mY2slxW4t2Y2SjC3&p=stories, I don't see any visual difference in delivery.

Is it possible that the change experienced here comes from https://github.com/adobe/spectrum-web-components/pull/3581/files#diff-0b816166dc6b4aa4bce7c3c01519dbc59034f4b63ab1d4b4beef48206355a851 instead? Here we caught up with some differences between out delivery of the Slider layout and that of CSS/the specification by bringing the label and the slider closer together.