-
Notifications
You must be signed in to change notification settings - Fork 58
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
When user clicks on the Edit button he should see action sheet with media options #532
Comments
How are we going to handle these types of situations on Android? Bottom Sheet? Menu? Dialog? I think my preference would be in that order – Bottom Sheet being most reachable, but Menu or Dialog wouldn't necessarily feel out of place. |
Hey @iamthomasbishop , please see how it's implemented |
Ok awesome – a Bottom Sheet :) Do we have control over which sheet options show per-platform? For example, the actions on Android and iOS are slightly different:
With that said I'm imagining the options per platform to be something along these lines: |
Just wanted to add that we should make an effort to lose the menu based approach when we get the chance. It's fine to keep it for ease of implementation for the February Beta but yeah, let's not get too cosy with it :) We managed to remove the so called 7-items monstrosity back in 2017 on Android and we need to plan some effort to go that route again. |
100% agree, this is a stop-gap solution until we have a new Media Picker functioning. |
Hey @iamthomasbishop , I have spoken with @SergioEstevao about further changes on Bottom Sheet. If we want to follow your proposal, it would need additional work on iOS side. We are sharing thoughts that we should stop here for beta as we have other tasks that we should resolve and let's continue after the beta release, WDYT ? |
@marecar3 and @iamthomasbishop just to make clear, what I was proposing was to limit the options for Choose From Device, Take Photo and Media Library. The other two while implemented in the main app still need the necessary hooks on the JS side. |
@SergioEstevao @marecar3 Thanks for clarifying. I think it makes sense if we have to limit the options just for the beta, although we should push to enable all of the options as soon as possible. For the sake of clarity, heres's what we're proposing for the beta: Note: I added icons on Android for the sake of mocking them up. We can exclude them for now, unless it's minimal work to add them. |
The text was updated successfully, but these errors were encountered: