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

HTML anatomy of items in the layers list needs refactoring #263

Closed
Malvoz opened this issue Jan 27, 2021 · 12 comments · Fixed by #531
Closed

HTML anatomy of items in the layers list needs refactoring #263

Malvoz opened this issue Jan 27, 2021 · 12 comments · Fixed by #531

Comments

@Malvoz
Copy link
Member

Malvoz commented Jan 27, 2021

HTML anatomy of a layer item

<fieldset aria-grabbed="false">
    <details class="mapml-control-layers">
        <summary>
            <label>
                <input checked="" type="checkbox" class="leaflet-control-layers-selector" />
                <span style="font-style: normal;"> Canada Base Map - Transportation (CBMT)</span>
            </label>
        </summary>
        <details class="mapml-control-layers">
            <summary>
                <label for="o82">Opacity</label>
            </summary>
            <input id="o82" type="range" min="0" max="1.0" value="1.0" step="0.1" />
        </details>
    </details>
</fieldset>

Nesting interactive elements inside <summary> is problematic:

Refactoring the layer items is not a small task. First we need to decide on a new layout that does not entail nesting interactive elements in <summary>, sigh.

@Malvoz
Copy link
Member Author

Malvoz commented Jan 29, 2021

Legend links (which I personally don't think should be a thing) and the "Remove Layer" buttons are also problematic since they're interactive elements inside summary.

@Malvoz

This comment has been minimized.

@Malvoz
Copy link
Member Author

Malvoz commented Mar 24, 2021

Proposal for alternative layer item anatomy:

<fieldset class="mapml-layer-item" aria-labelledby="mapml-layer-item-name-{1}" aria-grabbed="false">
  <div class="mapml-layer-item-properties">
    <label class="mapml-layer-item-toggle" title="Enable layer">
      <input type="checkbox" checked />
    </label>

    <span class="mapml-layer-item-name" id="mapml-layer-item-name-{1}">Layer 1</span>

    <div class="mapml-layer-item-controls">
      <button type="button" class="mapml-layer-item-remove-control mapml-button" title="Remove layer">
        <span class="mapml-button-icon" aria-hidden="true">
          <!-- icon (inline SVG or CSS background-image) -->
          <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" height="22" width="22">
            <path d="M0 0h24v24H0V0z" fill="none" />
            <path d="M18.3 5.71c-.39-.39-1.02-.39-1.41 0L12 10.59 7.11 5.7c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41L10.59 12 5.7 16.89c-.39.39-.39 1.02 0 1.41.39.39 1.02.39 1.41 0L12 13.41l4.89 4.89c.39.39 1.02.39 1.41 0 .39-.39.39-1.02 0-1.41L13.41 12l4.89-4.89c.38-.38.38-1.02 0-1.4z"/>
          </svg>
        </span>
      </button>

      <button type="button" class="mapml-layer-item-settings-control mapml-button" title="Layer settings" aria-expanded="false">
        <span class="mapml-button-icon" aria-hidden="true">
          <!-- icon (inline SVG or CSS background-image) -->
          <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" height="22" width="22">
            <path d="M0 0h24v24H0z" fill="none" />
            <path d="M12 8c1.1 0 2-.9 2-2s-.9-2-2-2-2 .9-2 2 .9 2 2 2zm0 2c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2zm0 6c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2z" />
          </svg>
        </span>
      </button>
    </div>
  </div>

  <div class="mapml-layer-item-settings" hidden>
    <details class="mapml-layer-item-opacity">
      <summary id="mapml-layer-item-opacity-name-{1}">Opacity</summary>
      <input type="range" min="0" max="1.0" value="1.0" step="0.1" aria-labelledby="mapml-layer-item-opacity-name-{1}" />
    </details>
  </div>
</fieldset>

The proposed anatomy:

  • does use <fieldset> (to allow disable of an entire layer list item, but it's not used as a flex container since that has interop issues)
  • ensures that each fieldset is properly labelled for ATs using ARIA
  • describes the purpose of each control for ATs
  • does not use nested interactive elements
  • does not use any pre-existing class names (although these may be improved) so does not rely on any existing styles for layer items
  • uses icons available under Apache license v2.0
Required CSS

(side note: builds on the generic mapml-button class to reset native styles of buttons.)

button.mapml-button:disabled,
.mapml-button[aria-disabled="true"],
.mapml-layer-item:disabled button.mapml-button {
  opacity: .3;
}

.mapml-button-icon {
  pointer-events: none;
}

.mapml-button-icon svg {
  fill: currentColor;
}

.mapml-layer-item,
.mapml-layer-item *,
.mapml-layer-item ::before,
.mapml-layer-item ::after {
  box-sizing: border-box;
}

.mapml-layer-item {
  border: 0;
  margin: 0;
  padding: 0;
}

.mapml-layer-item:not(:last-of-type) {
  border-bottom: 1px solid #e3e3e3;
}

.mapml-layer-item-properties {
  align-items: center;
  display: flex;
  justify-content: space-between;
}

.mapml-layer-item-controls {
  margin-inline-start: auto;
}

.mapml-layer-item-controls button svg {
  vertical-align: middle;
}

.mapml-layer-item-controls,
label.mapml-layer-item-toggle,
.mapml-layer-item-remove-control {
  align-items: center;
  display: flex;
  justify-content: center;
}

.mapml-layer-item-name {
  padding: .5rem;
  padding-inline-start: 0;
  word-break: break-word;
}

.mapml-layer-item-toggle,
.mapml-layer-item-remove-control,
.mapml-layer-item-settings-control {
  min-height: 44px;
  min-width: 44px;
  height: 44px;
  width: 44px;
}

.mapml-layer-item-settings > * {
  display: block;
  padding-block-start: .25rem;
  padding-block-end: .25rem;
  padding-inline-start: 44px;
  padding-inline-end: 20px;
}

.mapml-layer-item-settings summary {
  -webkit-user-select: none;
          user-select: none;
}

.mapml-layer-item-opacity [type="range"] {
  margin-inline-end: 0;
  margin-inline-start: 0;
  width: 100%;
}

The required styles:

Implementation details:

  • <fieldset> should be labelled by the layer name
  • <button class="mapml-layer-item-settings-control"> should default to aria-expanded="false"
  • <div class="mapml-layer-item-settings"> should default to hidden
  • When the <button class="mapml-layer-item-settings-control"> is pressed:
    • Its aria-expanded attribute is set to true
    • the corresponding <div class="mapml-layer-item-settings">'s hidden attribute is removed.
  • In <details class="mapml-layer-item-opacity"> the <input type="range"> should be labelled by <summary>

Result:

(from copy & pasting 2 example layer items into the existing layer control along with the required CSS)
<!-- Example disabled layer item -->
<fieldset class="mapml-layer-item" aria-labelledby="mapml-layer-item-name-{1}" aria-grabbed="false" disabled>
  <div class="mapml-layer-item-properties">
    <label class="mapml-layer-item-toggle" title="Enable layer">
      <input type="checkbox" checked />
    </label>

    <span class="mapml-layer-item-name" id="mapml-layer-item-name-{1}">Layer 1</span>

    <div class="mapml-layer-item-controls">
      <button type="button" class="mapml-layer-item-remove-control mapml-button" title="Remove layer">
        <span class="mapml-button-icon" aria-hidden="true">
          <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" height="22" width="22">
            <path d="M0 0h24v24H0V0z" fill="none" />
            <path d="M18.3 5.71c-.39-.39-1.02-.39-1.41 0L12 10.59 7.11 5.7c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41L10.59 12 5.7 16.89c-.39.39-.39 1.02 0 1.41.39.39 1.02.39 1.41 0L12 13.41l4.89 4.89c.39.39 1.02.39 1.41 0 .39-.39.39-1.02 0-1.41L13.41 12l4.89-4.89c.38-.38.38-1.02 0-1.4z"/>
          </svg>
        </span>
      </button>

      <button type="button" class="mapml-layer-item-settings-control mapml-button" title="Layer settings" aria-expanded="false">
        <span class="mapml-button-icon" aria-hidden="true">
          <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" height="22" width="22">
            <path d="M0 0h24v24H0z" fill="none" />
            <path d="M12 8c1.1 0 2-.9 2-2s-.9-2-2-2-2 .9-2 2 .9 2 2 2zm0 2c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2zm0 6c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2z" />
          </svg>
        </span>
      </button>
    </div>
  </div>

  <div class="mapml-layer-item-settings" hidden>
    <details class="mapml-layer-item-opacity">
      <summary id="mapml-layer-item-opacity-name-{1}">Opacity</summary>
      <input type="range" min="0" max="1.0" value="1.0" step="0.1" aria-labelledby="mapml-layer-item-opacity-name-{1}" />
    </details>
  </div>
</fieldset>

<!-- Example enabled layer item with expanded settings -->
<fieldset class="mapml-layer-item" aria-labelledby="mapml-layer-item-name-{2}" aria-grabbed="false">
  <div class="mapml-layer-item-properties">
    <label class="mapml-layer-item-toggle" title="Enable layer">
      <input type="checkbox" checked />
    </label>

    <span class="mapml-layer-item-name" id="mapml-layer-item-name-{2}">Layer 2, this has a longer name</span>

    <div class="mapml-layer-item-controls">
      <button type="button" class="mapml-layer-item-remove-control mapml-button" title="Remove layer">
        <span class="mapml-button-icon" aria-hidden="true">
          <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" height="22" width="22">
            <path d="M0 0h24v24H0V0z" fill="none" />
            <path d="M18.3 5.71c-.39-.39-1.02-.39-1.41 0L12 10.59 7.11 5.7c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41L10.59 12 5.7 16.89c-.39.39-.39 1.02 0 1.41.39.39 1.02.39 1.41 0L12 13.41l4.89 4.89c.39.39 1.02.39 1.41 0 .39-.39.39-1.02 0-1.41L13.41 12l4.89-4.89c.38-.38.38-1.02 0-1.4z"/>
          </svg>
        </span>
      </button>

      <button type="button" class="mapml-layer-item-settings-control mapml-button" title="Layer settings" aria-expanded="true">
        <span class="mapml-button-icon" aria-hidden="true">
          <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" height="22" width="22">
            <path d="M0 0h24v24H0z" fill="none" />
            <path d="M12 8c1.1 0 2-.9 2-2s-.9-2-2-2-2 .9-2 2 .9 2 2 2zm0 2c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2zm0 6c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2z" />
          </svg>
        </span>
      </button>
    </div>
  </div>

  <div class="mapml-layer-item-settings">
    <details class="mapml-layer-item-opacity">
      <summary id="mapml-layer-item-opacity-name-{2}">Opacity</summary>
      <input type="range" min="0" max="1.0" value="1.0" step="0.1" aria-labelledby="mapml-layer-item-opacity-name-{2}" />
    </details>
  </div>
</fieldset>

alt-layer-list-item-anatomy

Thoughts?

@prushforth
Copy link
Member

Thoughts?

@Malvoz we're going to try to address this issue. After, we want to try implementing multiple <extent> elements (and expose those extents as toggles/checkboxes in the layer control), but don't want to start that until we have got agreement on an accessible layer control structure.

Any advice you can give us beyond that you've already offered is much appreciated. Thank you!

@Malvoz
Copy link
Member Author

Malvoz commented Oct 4, 2021

I will say that the proposed layer item anatomy is probably not perfect from a design point of view, though it should resolve the current accessibility issues.

Also it did not take into consideration: #497, but none of our components currently do, so that should be fine.

@Malvoz
Copy link
Member Author

Malvoz commented Oct 4, 2021

After, we want to try implementing multiple <extent> elements (and expose those extents as toggles/checkboxes in the layer control), but don't want to start that until we have got agreement on an accessible layer control structure.

Are the toggles to be regarded as settings for a particular layer? The proposed anatomy allows for new types of settings as children of <div class="mapml-layer-item-settings">:

<div class="mapml-layer-item-settings">
  <!-- opacity settings -->
  <details class="mapml-layer-item-opacity">
    ...
  </details>
  <!-- new extent/projection settings can be added here (or before opacity, if it's deemed more important) -->
  <details class="mapml-layer-item-projections"><!-- or what you want to call it, also doesn't have to be <details> -->
    <!-- add anything you want here, e.g. input controls -->
  </details>
</div>

@prushforth
Copy link
Member

prushforth commented Oct 4, 2021

Are the toggles to be regarded as settings for a particular layer?

Not exactly. Currently, there is only one extent allowed / used per layer. When a layer is enabled, all the settings that we expose, for example, opacity or checked, apply to the whole layer, including all the templated links within the extent.

What we would like to achieve is the ability to treat individual extent elements as virtual child layers of the layer, each with its own individually settable settings, including opacity and checked. In effect: nested "layers" only one level of nesting deep.

The nesting, I think need only be visual, not marked up in the sense of nesting interactive elements being undesirable. We will need to think about how to achieve it in a user friendly way, but we would like the user to have ability to turn on/off individual extents (checked / unchecked), as well as set opacity for them and so on. Finally all of the extents in a layer should turn on/off or similarly respond to changes in settings of the parent <layer->, I think, tbd. Needs some thought.

@Anshpreet8
Copy link
Contributor

@Malvoz is it also not recommended to have details element nested within details?

@Malvoz
Copy link
Member Author

Malvoz commented Oct 5, 2021

@Anshpreet8 nesting details is fine, generally avoid nesting interactive content (including elements in the tab order due to tabindex).

Edit: while that's a bit contradictive since details is on that list, you can nest details: https://html5doctor.com/the-details-and-summary-elements/#:~:text=Nesting%20multiple%20%3Cdetails%3E%20elements (without causing issues for screen readers).

@Malvoz
Copy link
Member Author

Malvoz commented Oct 5, 2021

@prushforth re #263 (comment):

I suppose that means extents must be labelable, to expose them in a meaningful matter in the layer control?

@Malvoz
Copy link
Member Author

Malvoz commented Oct 5, 2021

Added label web platform compatibility per:

@prushforth
Copy link
Member

I suppose that means extents must be labelable, to expose them in a meaningful matter in the layer control?

I think so. I was thinking on using the <link title="foo" ...> title attribute, but maybe a label should be considered. There can/should be only one of those, whereas we can include many <link tref="..."> elements so designing a label for the extent seems appropriate.

Currently the label for the layer (and therefore the singular extent) is from the <layer- label="foo"> attribute.

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