Conversation
Changes:
Total lines: 4524 (+3) |
deployed preview: https://459.connect-d5y.pages.devWelcome to connect! Make sure to:
Mobile
Desktop
|
|
it works in the browser but not the PWA. seems intentional? there should be a better way than installing a random package |
|
Looks like we had a custom component for this in old connect - commaai/connect#249 I think they disable it on iOS PWA when you pick "standalone" to make it feel more like a native app |
|
Trying this PR on iOS/Safari, if I drag the page down it does overscroll, but it doesn't trigger a refresh. It also breaks scrolling the page normally.. |
|
I think I narrowed down the fix for Android to just one change |
|
Hm we added that class to fix another bug on iOS (commaai/connect#18). We could fix that by just making the drawer opacity 0 when it's supposed to be hidden. |
|
This is huge. First let's do the simplest thing on Android without regressing, then open another PR for iOS. This is not splitting PRs up |
incognitojam
force-pushed
the
fix-pull-to-refresh
branch
3 times, most recently
from
6b43235 to
ceaaee6
Compare
| /* prevent horizontal scrolling, e.g. when drawer is open */ | ||
| @apply overflow-x-hidden; |
There was a problem hiding this comment.
I don't notice this, what is it for?
There was a problem hiding this comment.
You'd have to test the branch as it was in be22dc8. We have to clip the overflow horizontally because we have a couple of animations which translate the UI horizontally:
- Opening the drawer slides it from left to right slightly, and the rest of the UI slides to the right
- The route activity slides away to the right when you go back to the route list
Without this you can scroll the whole page horizontally.
There was a problem hiding this comment.
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
Files not reviewed (1)
- src/index.css: Language not supported
Comments suppressed due to low confidence (1)
src/components/material/Drawer.tsx:52
- The change in opacity from 0.5 to 0 when the drawer is closed could affect the overscroll behavior; please verify that this does not unintentionally block necessary visual cues for pull-to-refresh.
opacity: drawerVisible() ? 1 : 0,
| // Opacity should be 0 when drawer is closed, otherwise it can be visible when overscrolling | ||
| opacity: drawerVisible() ? 1 : 0, |
There was a problem hiding this comment.
This is for the bug previously addressed in #18
we only need to allow vertical scrolling for pull down to refresh
This reverts commit bea6486.
incognitojam
force-pushed
the
fix-pull-to-refresh
branch
from
ceaaee6 to
38ff496
Compare
|
Even this change causes weirdness on iOS: when I open the drawer, if I swipe left I can scroll and see the dashboard again. I can't repro it on the deployed master. Not sure how not setting |
|
Can we merge something simple that works on Android? |
add pull to refresh in iOS standalone PWA