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

feat: sheet style modal breakpoints should be reactive #24752

Closed
3 tasks done
EinfachHans opened this issue Feb 9, 2022 · 18 comments
Closed
3 tasks done

feat: sheet style modal breakpoints should be reactive #24752

EinfachHans opened this issue Feb 9, 2022 · 18 comments
Labels

Comments

@EinfachHans
Copy link
Contributor

Prerequisites

Describe the Feature Request

The breakpoints of a sheet style modal should be reactive

Describe the Use Case

Our Sheet has some breakpoints that should be only available in a specific state

Describe Preferred Solution

The best solution would be a method setBreakpoint which takes the new breakpoints array and a new initalBreakpoint that is used only if the current breakpoint is not included in the new array.

Describe Alternatives

Just be able to set this.modal.breakpoints = [...]. The users then has to check by their self to maybe move to another breakpoint manually (via #24648)

Related Code

No response

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Feb 9, 2022
@liamdebeasi
Copy link
Contributor

Can you provide an example of the problem you are running into with the current behavior?

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

Reassign the breakpoints from within the Modal like this.modal.breakpoints = [...] doesn't work, because the Gesture is not recreated

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

I understand the issue, but what is the use case for re-configuring the breakpoints?

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

In our use case we have a Sheet with three different states. But depending on a specific condition (20 seconds left of an auction) we want to disable/skip one of these and only show the other two.

I may should add that the content of the sheet depends on the current breakpoint

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

liamdebeasi commented Feb 9, 2022

Thanks! Do you know of any native apps that have a similar behavior that we could reference?

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

Honestly no 🤔 But guess it's possible and would be easy to add for users building similar use cases as mine

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

Thanks. Can you tell me more about your use case in #24752 (comment)? I'm wondering if #22297 with a card modal would be a better UI for this.

Sheet modals are very dynamic/temporary, so restricting both of those seem to go against what the sheet is designed for.

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

The Use Case is more like the old GitHub Sheet Modal (that diesn't exist anymore)..

To our app is about auctions and bidding. If you open a Auction-Page we have that Sheet in lowest breakpoint state that contains informations: price, vat, ect. and two buttons: "instant bid" and "bid". If you click on "bid" the sheet extends to the middle breakpoint, where then also a input is visible to enter a custom amount of money. The last state also contains some information about the bid history then.

This use case is not working with a card style modal, because in lowest state the page should still be scrollable etc (via backdropBreakpoint this works perfect) and users should be able to extend the sheet.

Also the UI i should implement says that the middle state should not be available in the hot bid phase (last 20 seconds).

I can also share a link to out ui mockups about what we planned, but i cant post this public here. If you are interessted i could send this to you via discord @liamdebeasi ?

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Feb 10, 2022
@DwieDima
Copy link
Contributor

we also have a use case for our customer, where you go through an transaction (breakpoint 0.25) and more information (breakpoint 0.5) is only revealed, when user agrees to terms of service.

@liamdebeasi
Copy link
Contributor

@DwieDima Are there any native apps you can think of that do this?

I talked more with the team and we still feel that the card modal with #22297 is more appropriate given the dynamic nature of the sheet modal.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Feb 14, 2022
@ionitron-bot ionitron-bot bot removed the triage label Feb 14, 2022
@ruiminxu
Copy link

ruiminxu commented Feb 15, 2022

@liamdebeasi Hey, I think if you were to use apple map as an example and when you click on the search bar, the modal expand up reviewing more information. I think it is good to add something to change the breakpoint of the sheet modal. For instance, once a user click a button to set the breakpoint of the modal to 0.5 while the modal is currently at 0.1. The modal should bounch to 0.5.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Feb 15, 2022
@EinfachHans
Copy link
Contributor Author

@liamdebeasi what do you say to my message? Thats not possible with a card style modal

@DwieDima
Copy link
Contributor

@liamdebeasi i haven't found a native app which provides this functionality.
But if we were able to trigger modal position programmatically and also disable/enable drag gesture programmatically this would also be perfect.

Like @ruiminxu mentioned, in apple maps the sheet modal is set to 100% height, if you focus the searchbar. There is already a PR for that.

i think there was also a feature request for disabling the sheet modal gestures programmatically, but havent found it.

@liamdebeasi
Copy link
Contributor

Thanks for the issue. I discussed this with the team, and we do not think that this feature is a good fit for Ionic.

We think that limiting breakpoints goes against the intention of how the bottom sheet is designed to work as it is a very dynamic component. In other words, being able to snap to 0.5 at one instance but not at another instance seems to be a confusing experience. Additionally, we were unable to find examples of native apps that do this, so it is hard to gauge exactly how this feature should work.

In terms of alternatives, we have a couple ideas.

  1. Preventing a modal from being closed seems to be a common pain point here. This would be solved by feat: canLeave event for overlays #22297 and is something we are interested in adding to Ionic.
  2. Programmatically snapping to a breakpoint seems to be common as well. This would be solved by feat: sheet modal programmatically move to a breakpoint #23917 and is also something we are interested in adding.

I am going to close this. Thanks!

@EinfachHans
Copy link
Contributor Author

Hey @liamdebeasi , i would like to open that topic again, because i found an example in an native app. The App is "klarna". As you can see in the video: Sheet opens in a small loading state, than goes into the second state where you have to confirm something. As shown, there aren't any more breakpoints. If confirmed the sheet moves up (breakpoints changed again). That's what i need, beeing able to change the breakpoints while the sheet is open.

Upload.from.GitHub.for.iOS.MOV

@sean-perkins
Copy link
Contributor

@EinfachHans I've done some deep discovery into the dynamic breakpoints request and also analyzed your video; here are my findings:

  1. iOS released the bottom sheet API in iOS 15.0, so it is fairly new to the eco-system. Main implementations we are seeing are custom implementations (not making use of the built-in API). We can infer this because iOS only supports 2 breakpoint sizes, medium and large (full height modal).
  2. iOS does not appear to allow customizing the breakpoints (they call them detents) once the view is presented.

If you are familiar with Swift, you can play with this SwiftUI example I created: https://gist.github.com/sean-perkins/389d7f3445538edbe90441b283bd43ff

Reviewing the video frame-by-frame, it appears that when selecting the button to advance, they are presenting a new view on top of the existing sheet modal (animated: false). Here's a frame-to-frame comparison:

Screen Shot 2022-03-22 at 1 35 15 PM Screen Shot 2022-03-22 at 1 35 21 PM

Once that view is presented, they are manually animating it the remaining distance to make it a semi-cohesive transition.

Here's my concerns with implementing this pattern, regardless of what native is currently doing:

  1. If breakpoints are dynamic and your current breakpoint is not included in the updated breakpoints value, what is the expected behavior? Does it go to the next breakpoint? Does it go to the previous breakpoint?
  2. backdropBreakpoint and the current breakpoint have behavior tied to each other, that can effect the opacity and the enabled status of the backdrop. Similar use case to my first concern, if the breakpoints change and that modifies the current breakpoint, the backdrop could end up in an incorrect state.

Here is my recommendation on how to solve for this use case in your application:

Demo: https://stackblitz.com/edit/angular-favzx7?file=src%2Fapp%2Fapp.component.html

  1. Use nested sheet modals
  2. When the parent modal presents the new modal, dismiss the parent so that users cannot drag the sheet modal down to see the previous modal.
  3. Fine tune/control the available breakpoints, initial breakpoint, etc. on the nested modal to give it the same appearance as your video.

This results in:

  1. No API changes to Ionic
  2. No risks in breaking gestures, current breakpoint being set to undesired value, or backdrop issues
  3. Full control and customization to your implementation.

Let me know your thoughts after reviewing the demo and my evaluation.

@EinfachHans
Copy link
Contributor Author

@sean-perkins okay, it's sad that this is not possible on native, but the "workaround" you provided would not look good in our solution. Also the question you had about changing the breakpoints and the current one not beeing into the new breakpoints isn't really a thing ionic really need to think about. Just thro out a warning and the developers has to check that this not happens from their code 🤷🏼‍♂️

Let me describe one scenario in our app:

As said it's a app where you can bid on auctions. If you open an auction that you won but have not paid yet there is a sheet style modal with two breakpoints:

  1. Shows information about the price, net, date, invoice, etc.

  2. additionally Shows the iban where you need to send the money to and also a "confirm" button

If you press this confirm button we show a toast and the sheet should move to the lower breakpoint again (possible through our pr from 6.1 now) - but then the user should not be able to enter the upper breakpoint again because he already send the money...

And there are more scenarios like this 😅

i understand that it may not be able in native but i also think it's a really small code change for a really cool feature!

@ionitron-bot
Copy link

ionitron-bot bot commented Apr 22, 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 Apr 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants