feat(svelte): Improve small screeen behavior#63859
Conversation
86a488d to
c59b8b2
Compare
There was a problem hiding this comment.
I guess this won't happen on actual mobile devices but in the simulator the popovers caused some strange page shift as the library tries to fit them into the view. I thought it makes sense to disable them for now for small devices.
There was a problem hiding this comment.
Search input suggestions didn't properly resize to the available space as they were loaded asynchronously. Maybe there's a way that doesn't rely on mutation observer but it seems to work for now, and was a relatively small change.
There was a problem hiding this comment.
Without this the sidebar didn't shrink to the available screen size.
There was a problem hiding this comment.
Not sure if that is a good idea or not, but I also didn't want to add multiple properties just to influence styling. This seemed like a good compromise.
There was a problem hiding this comment.
Every time when someone uses a bitmask something good wakes up in my soul, thanks
There was a problem hiding this comment.
This was just moved from above as is.
There was a problem hiding this comment.
I don't know how I triggered this but I got an error that unsubscribe was accessed before it was assigned.
subscribe will call the callback synchronously so it's possible that the store already has a value that matches the conditions and unsubscribe is called before subscribe returned.
Moving this around fixes this issue.
There was a problem hiding this comment.
I added this to resize popovers vertically.
There was a problem hiding this comment.
Instead of having Popover hide on click when showOnHover is on, we do it manually here.
There was a problem hiding this comment.
This interferes with having popovers be conditionally by triggered by click or hover. I think callsites should be responsible for hiding the popover as necessary.
However I think we can probably make this more ergonomic and automatically enable click to show/hide behavior on mobile, if we have some more time.
camdencheek
left a comment
There was a problem hiding this comment.
This is so much more usable -- thank you!
There was a problem hiding this comment.
Should all keyboard shortcuts be hidden on mobile?
There was a problem hiding this comment.
Probably. I can move that into the keyboard shortcut component instead.
There was a problem hiding this comment.
this converts the opt out badge into a button that works on click on mobile (since hover is disabled).
There was a problem hiding this comment.
The changes here are quite "rudimentary". I don't know how they affect the panel system though it shouldn't matter much on mobile.
Still it would be nice to think about properly supporting this use case and not just tack it on.
|
Made some additional updates to make the revision picker work. For this I added an additional option to the default popover config that allows the popover logic to move the popover to the cross axis.
|
There was a problem hiding this comment.
Not sure if that's the best way to go about. It requires that we set grid-area: in the other component. But maybe that's OK since they are related.
There was a problem hiding this comment.
Is it impossible to have overflow by X axes? What is the reason for this change
There was a problem hiding this comment.
With the new size middleware the actual size of the popover might be different from what the popover content expects or there might simply not enough space.
E.g the content of the search results progress button was too large to completely show vertically. In this case the popover should be scrollable.
You are right that this might also be the case horizontally, although in this case I'd expect the popover content to simply shrink/wrap.
There was a problem hiding this comment.
Every time when someone uses a bitmask something good wakes up in my soul, thanks
There was a problem hiding this comment.
You split this into two strings, I'm just curious: Is this some new grid syntax? I thought that there should be no difference.
There was a problem hiding this comment.
Each string is a space separated row, so this means:
title timestamp
author author
This is an initial attempty to improve the web app for small screens.
This commit makes the following changes for small screens:
- Search home page:
- No more search input overflow
- History button is rendered on same line as other action buttons
(saves some vertical space)
- Search results page:
- Search input is rendered in the page, not in the header (more space)
- Filters sidebar is hidden by default and can be shown via a
`Filters` button.
- The filters sidebar opens fullscreen and has a close button
- The progrss button is smaller due to showing less information
- Repository pages:
- File sidebar is hidden by default. It can be shown via a new button
that is visible in the file headers
- The file sidebar opens fullscreen and has a close button
- NOTE: Selecting a file currently does not close the file sidebar,
navigation happens in the background
- Cody sidebar opens fullscreen and has the same close button
- General:
- Fuzzy finder opens fullscreen and has a larger close button
- Tabs don't show keyboard shortcuts
I tried to stick to CSS as much as possible but for some things to work
I had to change component structures or rendered elements conditionally.
Specifically when a component was already using `$isViewportMobile` I
usually just rendered elements conditionally.
I extended the `Panel` component to have a special 'mobile' mode, since
I realized I was doing similar changes to the filters, file tree and
cody sidebar.
The cody sidebar is a bit of a special case though because it's not
even rendered by default. So there are some additional steps required to
sync the open state.
e286f45 to
ef4fd34
Compare
ef4fd34 to
1eb94ef
Compare
|
I changed the keyboard shortcuts component to not render on mobile and update the history panel and diff view to look a bit better:
|
Closes srch-730
This is an initial attempt to improve the web app for small screens. This commit makes the following changes for small screens:
Filtersbutton.I tried to stick to CSS as much as possible but for some things to work I had to change component structures or rendered elements conditionally. Specifically when a component was already using
$isViewportMobileI usually just rendered elements conditionally.I extended the
Panelcomponent to have a special 'mobile' mode, since I realized I was doing similar changes to the filters, file tree and cody sidebar.The cody sidebar is a bit of a special case though because it's not even rendered by default. So there are some additional steps required to sync the open state.
Screenshots (iPhone SE, which is one of the smaller phones I guess)
Me rambling about the changes:
sg-sk-mobile.mp4
Note that the commits and commit pages already seem to look fine. The branches, tags and contributors pages are a bit broken due to use of tables and fixed columns. I can look at those separately.
Test plan
Manual testing