Skip to content

Commit

Permalink
Fix mobile navigation tabs expanding/collapsing when scrolling in mod…
Browse files Browse the repository at this point in the history 
…als (#3731)

* Update scroll provider so that it only captures the scroll on div container and not the whole window

* Fix lint + release notes

* Rewrite useScroll to be more performant by being ref based instead of state to avoid re-renders when scrolling

* Check undefined

* Rename to useScrollListener

* Remove small 1px gap under mobile nav tabs when fully open

* Cleanup

* Fix lint

* Coderabbit feedback
  • Loading branch information
@joel-jeremy
joel-jeremy authored Nov 17, 2024
1 parent dad702e commit e170c0d
Show file tree
Hide file tree
Showing 9 changed files with 362 additions and 209 deletions.
Binary file modified ...ts/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...ts/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...ts/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
49 changes: 23 additions & 26 deletions packages/desktop-client/src/components/App.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import { FinancesApp } from './FinancesApp';
import { ManagementApp } from './manager/ManagementApp';
import { Modals } from './Modals';
import { ResponsiveProvider } from './responsive/ResponsiveProvider';
import { ScrollProvider } from './ScrollProvider';
import { SidebarProvider } from './sidebar/SidebarProvider';
import { UpdateNotification } from './UpdateNotification';

Expand Down Expand Up @@ -180,36 +179,34 @@ export function App() {
<SidebarProvider>
<BudgetMonthCountProvider>
<DndProvider backend={HTML5Backend}>
<ScrollProvider>
<View
data-theme={theme}
style={{
height: '100%',
display: 'flex',
flexDirection: 'column',
}}
>
<View
data-theme={theme}
key={
hiddenScrollbars ? 'hidden-scrollbars' : 'scrollbars'
}
style={{
height: '100%',
display: 'flex',
flexDirection: 'column',
flexGrow: 1,
overflow: 'hidden',
...styles.lightScrollbar,
}}
>
<View
key={
hiddenScrollbars ? 'hidden-scrollbars' : 'scrollbars'
}
style={{
flexGrow: 1,
overflow: 'hidden',
...styles.lightScrollbar,
}}
>
<ErrorBoundary FallbackComponent={ErrorFallback}>
{process.env.REACT_APP_REVIEW_ID &&
!Platform.isPlaywright && <DevelopmentTopBar />}
<AppInner />
</ErrorBoundary>
<ThemeStyle />
<Modals />
<UpdateNotification />
</View>
<ErrorBoundary FallbackComponent={ErrorFallback}>
{process.env.REACT_APP_REVIEW_ID &&
!Platform.isPlaywright && <DevelopmentTopBar />}
<AppInner />
</ErrorBoundary>
<ThemeStyle />
<Modals />
<UpdateNotification />
</View>
</ScrollProvider>
</View>
</DndProvider>
</BudgetMonthCountProvider>
</SidebarProvider>
Expand Down
211 changes: 110 additions & 101 deletions packages/desktop-client/src/components/FinancesApp.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-strict-ignore
import React, { type ReactElement, useEffect } from 'react';
import React, { type ReactElement, useEffect, useRef } from 'react';
import { useTranslation } from 'react-i18next';
import { useDispatch, useSelector } from 'react-redux';
import {
Expand Down Expand Up @@ -34,6 +34,7 @@ import { Reports } from './reports';
import { LoadingIndicator } from './reports/LoadingIndicator';
import { NarrowAlternate, WideComponent } from './responsive';
import { useResponsive } from './responsive/ResponsiveProvider';
import { ScrollProvider } from './ScrollProvider';
import { Settings } from './settings';
import { FloatableSidebar } from './sidebar';
import { Titlebar } from './Titlebar';
Expand Down Expand Up @@ -156,6 +157,8 @@ export function FinancesApp() {
run();
}, [lastUsedVersion, setLastUsedVersion]);

const scrollableRef = useRef<HTMLDivElement>(null);

return (
<View style={{ height: '100%' }}>
<RouterBehaviors />
Expand All @@ -179,113 +182,119 @@ export function FinancesApp() {
width: '100%',
}}
>
<View
style={{
flex: 1,
overflow: 'auto',
position: 'relative',
}}
<ScrollProvider
isDisabled={!isNarrowWidth}
scrollableRef={scrollableRef}
>
<Titlebar
<View
ref={scrollableRef}
style={{
WebkitAppRegion: 'drag',
position: 'absolute',
top: 0,
left: 0,
right: 0,
zIndex: 1000,
flex: 1,
overflow: 'auto',
position: 'relative',
}}
/>
<Notifications />
<BankSyncStatus />

<Routes>
<Route
path="/"
element={
accountsLoaded ? (
accounts.length > 0 ? (
<Navigate to="/budget" replace />
>
<Titlebar
style={{
WebkitAppRegion: 'drag',
position: 'absolute',
top: 0,
left: 0,
right: 0,
zIndex: 1000,
}}
/>
<Notifications />
<BankSyncStatus />

<Routes>
<Route
path="/"
element={
accountsLoaded ? (
accounts.length > 0 ? (
<Navigate to="/budget" replace />
) : (
// If there are no accounts, we want to redirect the user to
// the All Accounts screen which will prompt them to add an account
<Navigate to="/accounts" replace />
)
) : (
// If there are no accounts, we want to redirect the user to
// the All Accounts screen which will prompt them to add an account
<Navigate to="/accounts" replace />
<LoadingIndicator />
)
) : (
<LoadingIndicator />
)
}
/>

<Route path="/reports/*" element={<Reports />} />
}
/>

<Route path="/reports/*" element={<Reports />} />

<Route
path="/budget"
element={<NarrowAlternate name="Budget" />}
/>

<Route
path="/schedules"
element={
<NarrowNotSupported>
<WideComponent name="Schedules" />
</NarrowNotSupported>
}
/>

<Route path="/payees" element={<ManagePayeesPage />} />
<Route path="/rules" element={<ManageRulesPage />} />
<Route path="/settings" element={<Settings />} />

<Route
path="/gocardless/link"
element={
<NarrowNotSupported>
<WideComponent name="GoCardlessLink" />
</NarrowNotSupported>
}
/>

<Route
path="/accounts"
element={<NarrowAlternate name="Accounts" />}
/>

<Route
path="/accounts/:id"
element={<NarrowAlternate name="Account" />}
/>

<Route
path="/transactions/:transactionId"
element={
<WideNotSupported>
<TransactionEdit />
</WideNotSupported>
}
/>

<Route
path="/categories/:id"
element={
<WideNotSupported>
<Category />
</WideNotSupported>
}
/>

{/* redirect all other traffic to the budget page */}
<Route path="/*" element={<Navigate to="/budget" replace />} />
</Routes>
</View>

<Route
path="/budget"
element={<NarrowAlternate name="Budget" />}
/>

<Route
path="/schedules"
element={
<NarrowNotSupported>
<WideComponent name="Schedules" />
</NarrowNotSupported>
}
/>

<Route path="/payees" element={<ManagePayeesPage />} />
<Route path="/rules" element={<ManageRulesPage />} />
<Route path="/settings" element={<Settings />} />

<Route
path="/gocardless/link"
element={
<NarrowNotSupported>
<WideComponent name="GoCardlessLink" />
</NarrowNotSupported>
}
/>

<Route
path="/accounts"
element={<NarrowAlternate name="Accounts" />}
/>

<Route
path="/accounts/:id"
element={<NarrowAlternate name="Account" />}
/>

<Route
path="/transactions/:transactionId"
element={
<WideNotSupported>
<TransactionEdit />
</WideNotSupported>
}
/>

<Route
path="/categories/:id"
element={
<WideNotSupported>
<Category />
</WideNotSupported>
}
/>

{/* redirect all other traffic to the budget page */}
<Route path="/*" element={<Navigate to="/budget" replace />} />
<Routes>
<Route path="/budget" element={<MobileNavTabs />} />
<Route path="/accounts" element={<MobileNavTabs />} />
<Route path="/settings" element={<MobileNavTabs />} />
<Route path="/reports" element={<MobileNavTabs />} />
<Route path="*" element={null} />
</Routes>
</View>

<Routes>
<Route path="/budget" element={<MobileNavTabs />} />
<Route path="/accounts" element={<MobileNavTabs />} />
<Route path="/settings" element={<MobileNavTabs />} />
<Route path="/reports" element={<MobileNavTabs />} />
<Route path="*" element={null} />
</Routes>
</ScrollProvider>
</View>
</View>
</View>
Expand Down
Loading

0 comments on commit e170c0d

Please sign in to comment.