feat(modal): clicking handle advances to the next breakpoint #25057
Conversation
| @@ -689,6 +689,21 @@ export class Modal implements ComponentInterface, OverlayInterface { | |||
| return this.currentBreakpoint; | |||
| } | |||
|
|
|||
| private onHandleClick = () => { | |||
| const { breakpoints, currentBreakpoint } = this; | |||
There was a problem hiding this comment.
Tabbing to the handle and pressing Enter multiple times causes the animation to jump.
Screen.Recording.2022-04-04.at.6.24.48.PM.mov
There was a problem hiding this comment.
Adjusted this behavior slightly, so that advancing is tied to keyup instead of keydown. Setting the current breakpoint does not currently wait for the previous transition to finish. If you perform an action while a transition is inflight, it will disrupt the transition.
There was a problem hiding this comment.
Looks like it's still jumping if you mash Enter; I feel like if there's a transition in progress when you click/activate the handle, it should just do nothing.
There was a problem hiding this comment.
@amandaesmith3 to confirm, if the user performs any action (clicking the handle or pressing enter), while the sheet modal is actively transitioning, the expected behavior would be to perform no operation (skip the interaction)?
There was a problem hiding this comment.
Looking back at the code, present and dismiss will both wait for the current transition and then go through with the attempted interaction, so we should probably go with that instead here. Thinking on it, if I hit Enter twice quickly, having it only move to the next breakpoint once would feel off. 🤔
There was a problem hiding this comment.
Waiting for the transition feels like it would conflict with the design document for the API of moving to a breakpoint:
If the user is in the middle of a gesture when this API is called, the user will be transitioned to the breakpoint value called by the API.
Unless we can cancel the current transition and immediately kick-off the new transition without a glitchy experience. I can do some more discovery on this.
There was a problem hiding this comment.
Updated the behavior, curious to both of your thoughts/feedback/alternatives. It now won't skip/jitter when you hold the enter key, but it will only performs moving to the next breakpoint when the current animation finishes. Hard to demonstrate in a recording.
There was a problem hiding this comment.
So when I try it, pressing Enter twice rapidly makes it only move once. It seems like it'll just eat your input if the animation is currently running. My guess is that since the current breakpoint doesn't actually update until after the animation finishes (triggered here), nextBreakpoint is calculated as the same thing for both Enter presses. Which would make the second press not really do anything.
That said, I'm okay with the behavior as-is. Up to you if you want to change it, though if we stick with it as-is, the code should probably be cleaned up to make it more clear.
|
Nice you tackled this 😊 Maybe it would be good to add a property to be able to disable this behaviour? There are apps that have this and others that don't have this… |
@EinfachHans oddly enough just had this exact conversation with the team 😆 we are going to hold on this PR until after 6.1.0 to write up a feature design document that covers the API additions and user behavior. Originally we anticipated this as a bug fix, but after comparing against other app patterns, we noticed that:
While this is a nice user enhancement, we want to make sure we get it right and offer the best configurations for developers to make use of it for their apps. |
|
Looking back at the code, present and dismiss will both wait for the[ current transition](text ) and then go through with the attempted interaction, so we should probably go with that instead here. Thinking on it, if I hit Enter twice quickly, having it only move to the next breakpoint once would feel off. |
|
Closing in favor of: #25540 |
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build) was run locally and any changes were pushednpm run lint) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
When clicking the handle inside a sheet modal, it does not advance to the next breakpoint.
Issue URL: #24069
What is the new behavior?
Clicking the handle will advance to the next breakpoint. If the user is at the last breakpoint, the sheet will advance to the first non-zero breakpoint.
While native iOS does have the ability to double tap the handle to dismiss the sheet modal (in certain experiences), we are not replicating that behavior in this feature. I believe that UX could be confusing and disruptive to users.
Does this introduce a breaking change?
Other information