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

bug: sheet modal gesture breaks when interacting with inputs/keyboard on ios #23878

Closed
4 of 6 tasks
EinfachHans opened this issue Sep 6, 2021 · 15 comments · Fixed by #24642
Closed
4 of 6 tasks

bug: sheet modal gesture breaks when interacting with inputs/keyboard on ios #23878

EinfachHans opened this issue Sep 6, 2021 · 15 comments · Fixed by #24642
Labels
package: core @ionic/core package type: bug a confirmed bug report v6 issues specific to Framework v6

Comments

@EinfachHans
Copy link
Contributor

Prequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

I just tested the new Sheet Modal. First: Great work 🎉 Here is a Collections of things i mentioned:

  • Similar to bug: Inline Modals not working #23561: My IDEA (InteliJ) seems to not know the new properties like breakpoints or initialBreakpoint. Maybe missing in the angular component declaration?
  • The handle can not be set to false for Sheet Modals:
    const showHandle = handle || isSheetModal;
  • The handle itself doesn't looks nice in combination with IonTitle - Looks like it needs a bit more spacing here
  • The Backdrop seems to be a bit buggy yet, see video: After drag finished it changes:
Bildschirmaufnahme.2021-09-06.um.13.27.54.mov
  • When i have an Input inside the Sheet Modal and focus it the Keyboard appears and the Modal pushs up a bit. After losing the focus of the input and the keyboard is gone, the breakpoints do not work anymore and i'm kind of in a free mode where i can stop the Modal wherever i want
  • Where we already talk about that: This freeMode would be a cool feature
  • Also it would be great to be able to disable the possibility to drag the Modal and add the possibility to change the current breakpoint programatically.

Expected Behavior

Everything is described above

Steps to Reproduce

Version: 6.0.0-beta.5

Code Reproduction URL

No response

Ionic Info

No response

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Sep 6, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Can you please provide reproductions for the buggy backdrop and the keyboard issues?

@EinfachHans
Copy link
Contributor Author

Of course. Here is the Repo: https://github.com/EinfachHans/sheet-modal-bugs

Just open the Modal, and move it in the states, the backdrop bug happens. Then tap into the input and tap out again. The freeMode is then enabled 🤪 (See Video)

Tested on Android 11 Galaxy 4 and iOS 14.7.1 (iPhone 11 Pro):

  • Backdrop Bug on both
  • FreeMode Bug only on iOS

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Sep 9, 2021
@liamdebeasi
Copy link
Contributor

Thanks. Looks like the backdrop issue happens when the breakpoints do not range from 0 to 1 (you had [0, 0.4, 0.7]. I need to check with the team to see if this is something we want to support. Do you have a use case for why you would want part of the modal hidden off screen?

I can reproduce the issue with the sheet not snapping, though I need to look into why that is happening.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Sep 9, 2021
@ionitron-bot ionitron-bot bot removed the triage label Sep 9, 2021
@EinfachHans
Copy link
Contributor Author

I would like to use the Sheet Modal for example single options, where the modal itself only contains one button. This should only have a specific height of for example 0.5, because expanding to 11 won't look really good.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Sep 9, 2021
@liamdebeasi
Copy link
Contributor

I think it would be better to set the height of the modal to 50% rather than have the height be 100% and have half of the modal hidden off screen:

<ion-modal [breakpoints]="[0, 1]">...</ion-modal>
ion-modal {
  height: 50%;
}

The breakpoints for ion-modal are relative to the height of the modal so that you do not need to re-adjust the breakpoints every time you change the height of the modal. (The docs for this are wrong, but that will be fixed next beta)

Can you try adjusting the height and let me know if that works?

@EinfachHans
Copy link
Contributor Author

This causes the modal to looks like this. Looks like it's top aligned somehow.

Bildschirmfoto 2021-09-09 um 15 18 04

@liamdebeasi
Copy link
Contributor

Oops sorry I meant --height: 50%. Can you try that instead?

@EinfachHans
Copy link
Contributor Author

Okay yes this works, but i mentioned something else: I guess in similar native setups there is kind of a bounce when you drag the modal. So basically you can drag the modal a bit higher as it is allowed to and if you release it snaps back. Now this feels very strict if you reached the 1 breakpoint

@liamdebeasi
Copy link
Contributor

Looks like the same issue as #22120 (impacts the card modal too)

@EinfachHans
Copy link
Contributor Author

Yes correct!

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Sep 14, 2021

Some updates on this:

  1. IDE issue is resolved via fix(modal): add sheet modal properties for angular #23899
  2. Handle position is resolved via fix(modal): sheet modal handle is now positioned correctly #23901
  3. Disabling handle is resolved via fix(modal): handle on sheet modal can now be turned off #23900
  4. Backdrop animation when breakpoints does not include 1 is resolved via fix(modal): sheet animation works correctly if breakpoints value does not include 1 #23927

The above 4 fixes will be available in an upcoming Ionic v6 beta.

The "bounce" you mentioned in #23878 (comment) will be tracked in #22120 since it also impacts card modals.
Programmatically moving the breakpoint is currently being discussed in another thread you have (#23917).

I am going to convert this thread to focus on the remaining issue which is the sheet gesture breaking when Interacting with the keyboard/inputs.

@liamdebeasi liamdebeasi changed the title v6 Sheet Modal bug: sheet modal gesture breaks when interacting with inputs/keyboard on ios Sep 14, 2021
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report v6 issues specific to Framework v6 labels Sep 14, 2021
@liamdebeasi liamdebeasi added this to the 6.0.0 milestone Sep 14, 2021
@ionitron-bot ionitron-bot bot removed the triage label Sep 14, 2021
@liamdebeasi liamdebeasi removed this from the 6.0.0 milestone Nov 23, 2021
@EinfachHans
Copy link
Contributor Author

@liamdebeasi why is this removed from the 6.0.0 Milestone? Guess thats a major issue 🤔

@liamdebeasi
Copy link
Contributor

We still have plans to fix, but the 6.0.0 milestone was outdated and not representative of what we are currently working on.

@EinfachHans
Copy link
Contributor Author

@liamdebeasi FYI, looks like the canStart of the sheet gesture is called when an input inside of the sheet style modal is clicked and the keyboard opens 🤔

Additional to that, i guess it would make sense to close open keyboards when start dragging by adjusting the canStart:

const canStart = (detail: GestureDetail) => {
  /**
   * If the sheet is fully expanded and
   * the user is swiping on the content,
   * the gesture should not start to
   * allow for scrolling on the content.
   */
  const content = (detail.event.target! as HTMLElement).closest('ion-content');

  if (currentBreakpoint === 1 && content) {
    return false;
  }

  const activeElement = baseEl.ownerDocument!.activeElement as HTMLElement;
  if (activeElement && activeElement.matches('input,ion-input, ion-textarea')) {
    activeElement.blur();
  }

  return true;
};

sean-perkins added a commit that referenced this issue Jan 24, 2022
sean-perkins added a commit that referenced this issue Jan 24, 2022
sean-perkins added a commit that referenced this issue Jan 24, 2022
sean-perkins added a commit that referenced this issue Jan 24, 2022
@ionitron-bot
Copy link

ionitron-bot bot commented Feb 24, 2022

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report v6 issues specific to Framework v6
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants