Skip to content
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

[Bugfix] Fixing Issue With Scrolling Client And Vendor Show Page #2362

Merged
merged 6 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/components/ScrollToTop.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,24 @@ import { useLocation } from 'react-router';
export function ScrollToTop(props: any) {
const location = useLocation();

const pathSegments = location.pathname.split('/');
const id = pathSegments?.[2] || '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you can extract id from the useParams?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beganovich I understand your wondering, but the reason for not having the possibility to extract it from useParams is because ScrollToTop is the parent of the whole <App />, which includes Router as well. Outside of the router concept, react-router-dom is not able to get parameters from the URL, and that is the only reason for it. If we want to achieve getting it with useParams, we would need to apply the ScrollToTop component to each route in the app. However, I think this approach, while not perfect, is not too bad since we just need that one check to see if something is included in the URL. Let me know your thoughts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just make sure this works both on hash and regular (browser) route then, please.

Copy link
Collaborator Author

@Civolilah Civolilah Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beganovich I just tested it, the same logic works for both "hash" and "browser" routers. I just did a tiny cleanup when checking if it is a client/vendor show page, but the logic with the ID remains the same.


const isClientShowPage =
id &&
location.pathname.startsWith(`/clients/${id}`) &&
!location.pathname.endsWith('/edit');

const isVendorShowPage =
id &&
location.pathname.startsWith(`/vendors/${id}`) &&
!location.pathname.endsWith('/edit');

useEffect(() => {
if (isClientShowPage || isVendorShowPage) {
return;
}

window.scrollTo(0, 0);
}, [location]);

Expand Down
19 changes: 19 additions & 0 deletions src/components/TabLoader.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Invoice Ninja (https://invoiceninja.com).
*
* @link https://github.com/invoiceninja/invoiceninja source repository
*
* @copyright Copyright (c) 2022. Invoice Ninja LLC (https://invoiceninja.com)
*
* @license https://www.elastic.co/licensing/elastic-license
*/

import { Spinner } from '$app/components/Spinner';

export function TabLoader() {
return (
<div className="py-4">
<Spinner />
</div>
);
}
102 changes: 90 additions & 12 deletions src/pages/clients/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import { assigned } from '$app/common/guards/guards/assigned';
import { or } from '$app/common/guards/guards/or';
import { permission } from '$app/common/guards/guards/permission';
import { Route } from 'react-router-dom';
import { lazy } from 'react';
import { lazy, Suspense } from 'react';
import { TabLoader } from '$app/components/TabLoader';

const Clients = lazy(() => import('$app/pages/clients/index/Clients'));
const Import = lazy(() => import('$app/pages/clients/import/Import'));
Expand Down Expand Up @@ -97,17 +98,94 @@ export const clientRoutes = (
/>
}
>
<Route path="" element={<Invoices />} />
<Route path="quotes" element={<Quotes />} />
<Route path="payments" element={<Payments />} />
<Route path="recurring_invoices" element={<RecurringInvoices />} />
<Route path="credits" element={<Credits />} />
<Route path="projects" element={<Projects />} />
<Route path="tasks" element={<Tasks />} />
<Route path="expenses" element={<Expenses />} />
<Route path="recurring_expenses" element={<RecurringExpenses />} />
<Route path="activities" element={<Activities />} />
<Route path="documents" element={<Documents />} />
<Route
path=""
element={
<Suspense fallback={<TabLoader />}>
<Invoices />
</Suspense>
}
/>
<Route
path="quotes"
element={
<Suspense fallback={<TabLoader />}>
<Quotes />
</Suspense>
}
/>
<Route
path="payments"
element={
<Suspense fallback={<TabLoader />}>
<Payments />
</Suspense>
}
/>
<Route
path="recurring_invoices"
element={
<Suspense fallback={<TabLoader />}>
<RecurringInvoices />
</Suspense>
}
/>
<Route
path="credits"
element={
<Suspense fallback={<TabLoader />}>
<Credits />
</Suspense>
}
/>
<Route
path="projects"
element={
<Suspense fallback={<TabLoader />}>
<Projects />
</Suspense>
}
/>
<Route
path="tasks"
element={
<Suspense fallback={<TabLoader />}>
<Tasks />
</Suspense>
}
/>
<Route
path="expenses"
element={
<Suspense fallback={<TabLoader />}>
<Expenses />
</Suspense>
}
/>
<Route
path="recurring_expenses"
element={
<Suspense fallback={<TabLoader />}>
<RecurringExpenses />
</Suspense>
}
/>
<Route
path="activities"
element={
<Suspense fallback={<TabLoader />}>
<Activities />
</Suspense>
}
/>
<Route
path="documents"
element={
<Suspense fallback={<TabLoader />}>
<Documents />
</Suspense>
}
/>
</Route>
<Route
path=":id/statement"
Expand Down
57 changes: 50 additions & 7 deletions src/pages/vendors/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import { enabled } from '$app/common/guards/guards/enabled';
import { ModuleBitmask } from '$app/pages/settings/account-management/component';
import { or } from '$app/common/guards/guards/or';
import { assigned } from '$app/common/guards/guards/assigned';
import { lazy } from 'react';
import { lazy, Suspense } from 'react';
import { TabLoader } from '$app/components/TabLoader';

const Vendors = lazy(() => import('$app/pages/vendors/index/Vendors'));
const Import = lazy(() => import('$app/pages/vendors/import/Import'));
Expand Down Expand Up @@ -80,12 +81,54 @@ export const vendorRoutes = (
/>
}
>
<Route path="" element={<PurchaseOrders />} />
<Route path="purchase_orders" element={<PurchaseOrders />} />
<Route path="expenses" element={<Expenses />} />
<Route path="recurring_expenses" element={<RecurringExpenses />} />
<Route path="activities" element={<Activities />} />
<Route path="documents" element={<Documents />} />
<Route
path=""
element={
<Suspense fallback={<TabLoader />}>
<PurchaseOrders />
</Suspense>
}
/>
<Route
path="purchase_orders"
element={
<Suspense fallback={<TabLoader />}>
<PurchaseOrders />
</Suspense>
}
/>
<Route
path="expenses"
element={
<Suspense fallback={<TabLoader />}>
<Expenses />
</Suspense>
}
/>
<Route
path="recurring_expenses"
element={
<Suspense fallback={<TabLoader />}>
<RecurringExpenses />
</Suspense>
}
/>
<Route
path="activities"
element={
<Suspense fallback={<TabLoader />}>
<Activities />
</Suspense>
}
/>
<Route
path="documents"
element={
<Suspense fallback={<TabLoader />}>
<Documents />
</Suspense>
}
/>
</Route>
<Route
path=":id/edit"
Expand Down
Loading