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
50.73k stars 13.52k forks source link

bug: interactive items in ion-item unexpectedly being focused with delegatesFocus #21982

Closed larsblumberg closed 4 months ago

larsblumberg commented 3 years ago

Bug Report

Ionic version: [ ] 4.x [x] 5.x

Current behavior: On Safari in macOS or iOS, IonInput (ion-input) steals focus when its value gets changed programmatically.

Expected behavior:

ion-input doesn't take the focus.

Steps to reproduce: The following code snippet reproduces the effect that when an ion-input’s value got changed progammatically, here via a button click, then the ion-input will receive focus automatically as an effect. This effect is only observable within an ion-item container.

Apart from the fact that this might be unwanted and thus should be controllable by the programmer, it also comes with the side effect of the virtual keyboard popping up since the input receives focus. That’s only acceptable if the user sets focus explicitely on the input. However in this case, the input surprisingly "steals" the focus.

Related code:

const Test = () => {
  const [value, setValue] = useState("")
  return <IonItem>
    <IonInput value={value} onIonChange={e => setValue(e.detail.value)} />
    <IonButton onClick={() => setValue("foo")}>foo</IonButton>
  </IonItem>
}

Full demo project available here: https://github.com/larsblumberg/ion-input-focus-stealing-on-safari

Other information:

Effect only observable when input is contained within an ion-item and only in Safari on macOS and iOS.

Ionic info:

Ionic:
   Ionic CLI       : 6.11.1 (/Users/lars/.config/yarn/global/node_modules/@ionic/cli)
   Ionic Framework : @ionic/react 5.2.2

Capacitor:
   Capacitor CLI   : 2.0.2
   @capacitor/core : 2.2.0

Utility:
   cordova-res : not installed
   native-run  : not installed

System:
   NodeJS : v14.4.0 (/usr/local/Cellar/node/14.4.0/bin/node)
   npm    : 6.14.4
   OS     : macOS Catalina
liamdebeasi commented 3 years ago

Thanks for the issue. Can you reproduce this in a blank Ionic starter app and provide a link to the GitHub repo?

ionitron-bot[bot] commented 3 years ago

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please provide a reproduction with the minimum amount of code required to reproduce the issue. Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

For a guide on how to create a good reproduction, see our Contributing Guide.

larsblumberg commented 3 years ago

Here's a full Ionic app which reproduces this issue on Safari in macOS and iOS: https://github.com/larsblumberg/ion-input-focus-stealing-on-safari

larsblumberg commented 3 years ago

@liamdebeasi If you have any workaround in mind that I could apply, that would be helpful. Due to this issue our iOS app has this awkward behavior that the virtual keyboard opens when the button is tapped (which changes an input value).

liamdebeasi commented 3 years ago

Apologies for the delay. This appears to be related to https://github.com/ionic-team/ionic-framework/issues/22037 (same underlying issue). We have a PR open here: https://github.com/ionic-team/ionic-framework/pull/22049 Though it needs to be updated to address buttons in addition to inputs.

The reason this is happening is ion-item has a Shadow DOM property call "delegatesFocus" set. Whenever ion-item receives focus, it will forward that focus to the first focusable element. In this case, clicking the button causes ion-item to receive focus, resulting in the focus being forwarded to the input.

larsblumberg commented 3 years ago

thanks for the update

larsblumberg commented 3 years ago

I've upgrade this issue's demo app to @ionic/react v. 5.3.3 as its release document states that the other, related issue https://github.com/ionic-team/ionic-framework/issues/22037 (linked by @liamdebeasi) addresses the same underlying bug. However, the behavior I'm describing here isn't fixed and it's still an issue (bug).

liamdebeasi commented 3 years ago

I can reproduce. Looks like we need to account for other focusable elements besides ion-input and ion-textarea as well. cc @brandyscarney

brandyscarney commented 3 years ago

@liamdebeasi So this was definitely introduced by 5.1.1 aka adding delegatesFocus to item in order to fix tabindex not properly being passed down, but I don't think it's related to the code here: https://github.com/ionic-team/ionic-framework/blob/d03807a44353af2b2906c6bcbafc2ccadaee3aa3/core/src/components/item/item.tsx#L180-L212

That code was added because the input was "kind of" getting focus but not really - the keyboard was opening when the item was pressed but the input didn't have focus. So that just made it so the input actually received the focus it was supposed to have.

In my testing of this issue, the input is getting focus only on the first button click, only when the value changes, and only in Safari. This always evaluates to false since the event target is the ion-button so it isn't actually hitting this at all: https://github.com/ionic-team/ionic-framework/blob/d03807a44353af2b2906c6bcbafc2ccadaee3aa3/core/src/components/item/item.tsx#L191

Has this been tested with iOS 14? I think it may be another webkit bug with delegatesFocus.

Let me know if I'm misunderstanding this!

larsblumberg commented 3 years ago

Has this been tested with iOS 14?

On iOS 14.0 the bug still manifests.

As well as on MacOS, Safari 14.0

xclusive36 commented 3 years ago

To force ion-input to not steal focus, Set ion-button with attribute tabIndex="1", Set ion-input with attribute tabIndex="2"

<IonItem> <IonInput tabIndex={2} value={value} onIonChange={e => setValue(e.detail.value)} /> <IonButton tabIndex={1} onClick={() => setValue("foo")}>foo</IonButton> </IonItem>

As a side effect, the ion-button will take on a border when focused. Add to css to remove the border.

ion-button:focus{ outline: none; }

arnotixe commented 3 years ago

Same here, angular 10, capacitor+ionic5. Clicklistener on ion-avatar silently fails (is not trigged) when sticking an ion-input into the same ion-item as the ion-avatar. Screenshot: image

ghost commented 3 years ago

Running Ionic React v5.5.4 and am still seeing this issue with textareas.

I have a page with a text input and a text area, and clicking between the two items I see the focus jump back to the textarea. It almost looks like there is a delay in the focus in the textarea

ghwilley commented 2 years ago

Running Ionic Angular 5.4.16 and seeing this issue with ion-inputs. It's within an ion-reorder-group, so there's no good way to avoid using ion-items.

Edit: I ended up getting around this by adding tabIndex to the elements that should keep focus.

Suxsem commented 2 years ago

@liamdebeasi any news? thank you!

grantmk commented 11 months ago

Same issue ion Mac / Safari. Due to following structure, clicking anywhere in the content area would snap focus to the input. Also meant user couldn't select and copy text in the content area (and we know that users hate being restricted from using your app in ways you didn't intend...)

<ion-reorder-group>
  <ion-item>
    <ion-card>
      <ion-input>
      <content>. <!-- Clicking anywhere in here (including buttons) snaps focus to ion-input above -->

Solved this by adding tabindex="0" to the container card:

<ion-card tabindex="0">

The only difference for the user is that keyboard tabs now include the whole card on the way to actual inputs. Can't foresee any other issues but happy to be corrected if there are.

alexbainbridge commented 10 months ago

Another workaround is change ion-item to a div with display:flex

liamdebeasi commented 4 months ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/29091, and a fix will be available in an upcoming major release of Ionic Framework.

ionitron-bot[bot] commented 3 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.