-
Notifications
You must be signed in to change notification settings - Fork 19
Implement Gabi feedback #812
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
Conversation
@nick1udwig ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make sure mobile is the first target. These changes break a number of things on mobile:
- Initial splash screen is good, but the copy talks about a desktop. Either rewrite or get input from bzns about how to rewrite this to focus on mobile first
- Splash screen modal "X" button is behind the right side bar on mobile
- Remove the "mailto" button. I'm happy to re-add this as a "DM using Official Chat App" button in future, but email is weird when considering a mobile first experience
- The "Edit" button looks good but when clicked the buttons overrun mobile screen (I'm looking at iPhone 12 Pro dimensions)
- I like this "link to app store if search not found", but really what search should be doing is opening the "All Apps" pane and searching therein. If no app is found, display that link
- I'm not opposed to the recents/home button on the right, but we need to keep in mind we are mobile first: these buttons are pretty small and hard to click on mobile. That's OK for now, though we should think if we can figure out how to improve this. The REAL problem is that the right side bar doesn't work and neither do the buttons. They must work AT LEAST as well as master (ideally better) before merge
- Dock overruns mobile width (I see "recent apps" going off screen)
- While we're here, when you click/tap anywhere in the "recent apps" view other than a button or an app, we should go home
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice. I like the draggable action button and it works well. We'll ask for more opinions when we get this into staging (so its easy for people to compare).
Let's fix this before merge:
- The draggable action button should remember where it was placed for the next session (can just use browser storage)
Unrelated nits that you can either fix here or make issues for later as your prefer:
- When in "app switch" view (i.e. after pressing the draggable button) on mobile I'd like to be able to drag any app left/right to close the app (current behavior is I have to click the
X
in top-right corner to close an app; this would be an additional way to close an app) - When tap "Apps" on mobile, should NOT focus the Search entry box; on desktop is good; when clicking into Search from homepage is good; but when tap "Apps" on mobile, focusing Search entry box opens keyboard which is annoying
- When in "Apps" view, clicking/tapping anywhere non-app/search/action-button should return to home (similar to in the "app switch" view tapping anywhere returns home)
- Get rid of the "Close" button on "Apps" view
- Get rid of the "Home"/"Close" buttons on "app switch" view
- On desktop, focusing the "Search apps" button behavior is correct, but the visual experience is weird because the search bar jumps from ~middle of screen to ~right of screen. The ideal case is the search bar looks the same in the two views
Beautiful. Really nice work. Love the dot! |
Problem
https://docs.google.com/presentation/d/1ksSf1m-LT6tH6GfJ0QO7MOFAhzvuegyX7HYp3UrtoqA/edit?slide=id.g372575dd330_0_73#slide=id.g372575dd330_0_73
Solution
Do the needful
Testing