ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.07k stars 13.51k forks source link

feat: add ability to prevent first interactive element inside ion-item from being automatically clicked when ion-item clicked. #28512

Closed ihardz closed 9 months ago

ihardz commented 12 months ago

Prerequisites

Describe the Feature Request

Add an appropriate property to disable auto-click first interactive element.

Describe the Use Case

it will give an ability to place an ion-toggle component inside of the menu item in 'sidemenu' template app so that it will be independent from the item click.

Describe Preferred Solution

add interactive prop into the ion-item component (probably with default value of true to avoid documentation change) it can be changed in the core/components/Item of your repo: See comments '// HERE_1' and '// HERE_2' in the code below.

<...>
getFirstInteractive() {
    if (Build.isTesting) {
      /**
       * Pseudo selectors can't be tested in unit tests.
       * It will cause an error when running the tests.
       *
       * TODO: FW-5205 - Remove the build conditional when this is fixed in Stencil
       */
      return undefined;
    }
    const controls = this.el.querySelectorAll('ion-toggle:not([disabled]), ion-checkbox:not([disabled]), ion-radio:not([disabled]), ion-select:not([disabled])');   // HERE_1
    return controls[0];
  }
<...>
render() {
    const { counterString, detail, detailIcon, download, fill, labelColorStyles, lines, disabled, href, rel, shape, target, routerAnimation, routerDirection, inheritedAriaAttributes, multipleInputs, } = this;
    const childStyles = {};
    const mode = getIonMode(this);
    const clickable = this.isClickable();
    const canActivate = this.canActivate();
    const TagType = clickable ? (href === undefined ? 'button' : 'a') : 'div';
    const attrs = TagType === 'button'
      ? { type: this.type }
      : {
        download,
        href,
        rel,
        target,
      };
    let clickFn = {};
    const firstInteractive = this.getFirstInteractive();
    // Only set onClick if the item is clickable to prevent screen
    // readers from reading all items as clickable
    if (clickable || (firstInteractive !== undefined && !multipleInputs)) {
      clickFn = {
        onClick: (ev) => {
          if (clickable) {
            openURL(href, ev, routerDirection, routerAnimation);
          }
          if (firstInteractive !== undefined && !multipleInputs) { // HERE_2
            const path = ev.composedPath();
            const target = path[0];
            if (ev.isTrusted) {
              /**
               * Dispatches a click event to the first interactive element,
               * when it is the result of a user clicking on the item.
               *
               * We check if the click target is in the shadow root,
               * which means the user clicked on the .item-native or
               * .item-inner padding.
               */
              const clickedWithinShadowRoot = this.el.shadowRoot.contains(target);
              if (clickedWithinShadowRoot) {
                firstInteractive.click();
              }
            }
          }
        },
      };
    }
<...>

Describe Alternatives

No response

Related Code

sidemenu template > App.vue :

<template>
  <ion-app>
    <ion-split-pane content-id="main-content">
      <ion-menu content-id="main-content" type="overlay">
        <ion-content>
          <ion-list id="inbox-list">
            <ion-list-header>Inbox</ion-list-header>
            <ion-note>hi@ionicframework.com</ion-note>

            <ion-menu-toggle :auto-hide="false" v-for="(p, i) in appPages" :key="i">
              <ion-item button detail @click="selectedIndex = i" router-direction="root" :router-link="p.url" lines="none" class="hydrated" :class="{ selected: selectedIndex === i }">
                <ion-icon aria-hidden="true" slot="start" :ios="p.iosIcon" :md="p.mdIcon"></ion-icon>
                <ion-label>{{ p.title }}</ion-label>
                <ion-toggle aria-label="Enable Section" slot="end"></ion-toggle>
              </ion-item>
            </ion-menu-toggle>
          </ion-list>

          <ion-list id="labels-list">
            <ion-list-header>Labels</ion-list-header>

            <ion-item v-for="(label, index) in labels" lines="none" :key="index">
              <ion-icon aria-hidden="true" slot="start" :ios="bookmarkOutline" :md="bookmarkSharp"></ion-icon>
              <ion-label>{{ label }}</ion-label>
            </ion-item>
          </ion-list>
        </ion-content>
      </ion-menu>
      <ion-router-outlet id="main-content"></ion-router-outlet>
    </ion-split-pane>
  </ion-app>
</template>

Additional Information

feel free to let me know if you would like I create a PR for this feature. Thank you!

sean-perkins commented 12 months ago

Hello @ihardz thanks for this feature request!

Can you provide additional information for the user experience? Do you an example iOS or Android application that uses this pattern that we can evaluate? The code samples are useful, but for features we heavily evaluate based on native UI patterns and accessible experiences.

Rendering an interactive element (toggle) inside of an item used as a button is an accessibility anti-pattern and will result in AXE violation warnings for using nested interactive elements.

ihardz commented 12 months ago

@sean-perkins, Samsung device > Settings > Advanced features > One-handed mode option: image

ihardz commented 12 months ago

@sean-perkins The feature request makes it possible. Does this make sense?

Upd: you can click on the label separately. It navigates to next screen. As well as toggle the feature

Android 13. One UI 5.1

liamdebeasi commented 12 months ago

@ihardz How does that work with a screen reader enabled (such as TalkBack)? In other words, are you able to select and activate the toggle and the label separately?

ihardz commented 11 months ago

@liamdebeasi , yes, i'm able to select and activate the toggle and the label separately

https://github.com/ionic-team/ionic-framework/assets/31627738/527a9e65-8d18-44e7-867e-91761ae8c199

liamdebeasi commented 11 months ago

Is this with TalkBack enabled though? Usually there's a focus ring around the focused element.

ihardz commented 11 months ago

@liamdebeasi , It was challenging, so sorry for delay. but i did it. It is still possible to select/activate them separately

Thanks Samsung for placing the same type item right on TalkBack menu screen. 😆

https://github.com/ionic-team/ionic-framework/assets/31627738/b5ac6b7e-4327-4cc7-8c66-d4293c2e7121

ihardz commented 11 months ago

@liamdebeasi , On Samsung devices, every menu item that has a vertical separator before the toggle element works like this. And the Toggle is inside the Item

sean-perkins commented 11 months ago

@ihardz I appreciate the video recording! Are you able to record the audio of the voice over with the experience as well? Based on the focused element, this appears to be a nested interactive (the entire container is interactive, but focus can then move to the toggle descendent).

iOS is inconsistent in their implementation of items with toggles, but they do not allow a separate interaction between the item and the toggle (other than force/long press, which is an OS interaction).

There are two variants I've identified in iOS, one where the label is not interactive and the toggle is and the other is when tapping the label activates the toggle:

iOS design where label toggles the toggle component

With the lack of consistent implementation across MD/iOS, the risk of a poor experience for screen reader users and also the architectural constraints of refactoring the ion-item implementation to render the slots outside of the interactive container, I don't recommend that this feature be introduced into Ionic. However, I'll wait to see if the VoiceOver experience with audio indicates anything I am be missing here.

ihardz commented 11 months ago

@sean-perkins , ok. with audio:

https://github.com/ionic-team/ionic-framework/assets/31627738/b98428c1-4b7f-4226-b5c8-d1ed63acb6e3

ihardz commented 11 months ago

@sean-perkins , @liamdebeasi , thoughts?

ihardz commented 11 months ago

@sean-perkins , @liamdebeasi, Do you plan to consider this feature be introduced into Ionic? Or Ionic is more for IOS style apps?

sean-perkins commented 11 months ago

Hello @ihardz!

After discussion with the team we are going to label this with "community feedback wanted". Mainly we want to understand how many developers would find benefit from this feature, before agreeing to include it in our backlog.

The proposed feature does exist within Material Design's spec (MD2): https://m2.material.io/components/lists#anatomy (search for "Secondary Action area"). However, the current architecture of ion-item is very problematic with supporting another interactive element from the slots, since the slots are part of a button element in the shadow DOM. Reworking this would be a significant effort to maintain the same style appearance.

I do think Android's setting interface is an example of a nested interactive that we would not support. This feature for allowing for multiple interactive elements would result in two tappable areas, the primary area would be the majority of space including the label and the other would be the slotted interactive to the "end" slot.

Thank you for this feature suggestion!

ionitron-bot[bot] commented 11 months ago

This issue has been labeled as community feedback wanted. This label is added to issues that we would like to hear from the community on before moving forward with any final decision on the feature request.

If the requested feature is something you would find useful for your applications, please react to the original post with 👍 (+1). If you would like to provide an additional use case for the feature, please post a comment.

The team will review this feedback and make a final decision. Any decision will be posted on this thread, but please note that we may ultimately decide not to pursue this feature.

Thank you!

liamdebeasi commented 9 months ago

Hey there,

We have not had sufficient feedback from the community, so we do not plan to move forward with the feature request at this time. I am going to close this, but we are happy to re-evaluate this decision if there is more support from the community in the future. Thank you!

ionitron-bot[bot] commented 8 months ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.