Fix #19068: While user in Pagination, Browser Back button takes them to start page #19229

shrushti2000 · 2025-01-03T14:13:52Z

Fixes #19068

Screen.Recording.2025-01-03.at.7.39.54.PM.mov

Type of change:

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

github-actions · 2025-01-03T14:14:15Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

github-actions · 2025-01-03T14:46:12Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

This reverts commit a585f3a.

github-actions · 2025-01-03T15:21:30Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

github-actions · 2025-01-06T15:14:38Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

chirag-madlani · 2025-01-03T14:18:43Z

...adata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx

+    cursorValue: string;
+    currentPage: number;
+  }>();
+  const currentPath = window.location.pathname;


instead use useCustomLocation

chirag-madlani · 2025-01-07T04:39:21Z

...adata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx

+          [cursorState.cursorType]: cursorState.cursorValue,
+          limit: pageSize,
+        });
+        handlePageChange(cursorState.currentPage as number);


I think we can pass cursor value to page handler and make it work as optional param so if we want to store it we can pass it else it won't be in effect

Just to clarify once, do you mean:
Passing cursorValue as an optional parameter in tablePaginationHandler?

const tablePaginationHandler = useCallback( ({ cursorType, currentPage, cursorValue}: PagingHandlerParams) => { if (cursorType && cursorValue) { getSchemaTables({ [cursorType]: paging[cursorType] }); storeCursor({ cursorType: cursorType, cursorValue: cursorValue, currentPage: currentPage, }); } handlePageChange(currentPage); }, [paging, getSchemaTables, storeCursor, handlePageChange] );

I am pointing at this: handlePageChange

handlePageChange is simply a setter function for currentPage in usePaging
handlePageChange: setCurrentPage

chirag-madlani · 2025-01-07T04:40:16Z

...adata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx

@@ -93,6 +94,7 @@ const DatabaseSchemaPage: FunctionComponent = () => {
  const { t } = useTranslation();
  const { getEntityPermissionByFqn } = usePermissionProvider();
  const pagingInfo = usePaging(PAGE_SIZE);
+  const { storeCursor, getStoredCursor } = usePaging();


We will be having both in line no. 96 no?
Also we are not passing default values from location?

github-actions · 2025-01-07T06:19:56Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

chirag-madlani · 2025-01-07T06:27:42Z

openmetadata-ui/src/main/resources/ui/src/hooks/paging/usePaging.ts

+interface CursorState {
+  cursorType: CursorType | null;
+  cursorValue: string | null;
+  currentPage: number | null;


currentPage is already there so no need to pull it here sepratly

chirag-madlani · 2025-01-07T06:28:46Z

openmetadata-ui/src/main/resources/ui/src/hooks/paging/usePaging.ts

+  storeCursor: (params: {
+    cursorType: CursorType | null;
+    cursorValue: string | null;
+    currentPage: number | null;
+  }) => void;
+  getStoredCursor: () => CursorState;


Let's follow same naming as above adn rename getter setter as below
handlePagingCursorChange
pagingCursor

chirag-madlani · 2025-01-07T06:29:42Z

openmetadata-ui/src/main/resources/ui/src/hooks/paging/usePaging.ts

+  const storeCursor = useCallback(
+    ({
+      cursorType,
+      cursorValue,
+      currentPage,
+    }: {
+      cursorType: CursorType | null;
+      cursorValue: string | null;
+      currentPage: number | null;
+    }) => {
+      history.replace({
+        state: {
+          cursorType,
+          cursorValue,
+          currentPage,
+        },
+      });
+    },
+    []
+  );


This function is not storing the cursor it's actually updating location state let's rename something to convey the same

chirag-madlani · 2025-01-07T06:30:58Z

openmetadata-ui/src/main/resources/ui/src/hooks/paging/usePaging.ts

+    return cursorState
+      ? {
+          cursorType: cursorState.cursorType || null,
+          cursorValue: cursorState.cursorValue || null,
+          currentPage: cursorState.currentPage || null,
+        }
+      : { cursorType: null, cursorValue: null, currentPage: null };


Suggested change

return cursorState

? {

cursorType: cursorState.cursorType || null,

cursorValue: cursorState.cursorValue || null,

currentPage: cursorState.currentPage || null,

}

: { cursorType: null, cursorValue: null, currentPage: null };

return {

cursorType: cursorState.cursorType || null,

cursorValue: cursorState.cursorValue || null,

currentPage: cursorState.currentPage || null,

};

chirag-madlani · 2025-01-07T06:31:50Z

...adata-ui/src/main/resources/ui/src/pages/DatabaseSchemaPage/DatabaseSchemaPage.component.tsx

+          [cursorState.cursorType]: cursorState.cursorValue,
+          limit: pageSize,
+        });
+        handlePageChange(cursorState.currentPage as number);


I am pointing at this: handlePageChange

github-actions · 2025-01-07T08:36:25Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

fix browser back btn issue related to pagination

456d79f

shrushti2000 had a problem deploying to test January 3, 2025 14:13 — with GitHub Actions Failure

shrushti2000 had a problem deploying to test January 3, 2025 14:14 — with GitHub Actions Failure

common function to handle cursor based data fetches

a585f3a

shrushti2000 had a problem deploying to test January 3, 2025 14:45 — with GitHub Actions Failure

Revert "common function to handle cursor based data fetches"

d3539af

This reverts commit a585f3a.

shrushti2000 had a problem deploying to test January 3, 2025 15:21 — with GitHub Actions Failure

updated usePaging hook to store and retrieve cursor

43e4b32

shrushti2000 had a problem deploying to test January 6, 2025 15:14 — with GitHub Actions Failure

chirag-madlani reviewed Jan 7, 2025

View reviewed changes

address PR comments

b6deece

shrushti2000 had a problem deploying to test January 7, 2025 06:19 — with GitHub Actions Failure

chirag-madlani reviewed Jan 7, 2025

View reviewed changes

address pr comments

f55eeeb

shrushti2000 had a problem deploying to test January 7, 2025 07:57 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #19068: While user in Pagination, Browser Back button takes them to start page #19229

Fix #19068: While user in Pagination, Browser Back button takes them to start page #19229

shrushti2000 commented Jan 3, 2025

github-actions bot commented Jan 3, 2025

github-actions bot commented Jan 3, 2025

github-actions bot commented Jan 3, 2025

github-actions bot commented Jan 6, 2025

chirag-madlani Jan 3, 2025

shrushti2000 Jan 7, 2025

chirag-madlani Jan 7, 2025

shrushti2000 Jan 7, 2025

chirag-madlani Jan 7, 2025

shrushti2000 Jan 7, 2025

chirag-madlani Jan 7, 2025

shrushti2000 Jan 7, 2025

github-actions bot commented Jan 7, 2025

chirag-madlani Jan 7, 2025

chirag-madlani Jan 7, 2025

shrushti2000 Jan 7, 2025

chirag-madlani Jan 7, 2025

shrushti2000 Jan 7, 2025

chirag-madlani Jan 7, 2025

shrushti2000 Jan 7, 2025

chirag-madlani Jan 7, 2025

github-actions bot commented Jan 7, 2025

Fix #19068: While user in Pagination, Browser Back button takes them to start page #19229

Are you sure you want to change the base?

Fix #19068: While user in Pagination, Browser Back button takes them to start page #19229

Conversation

shrushti2000 commented Jan 3, 2025

Type of change:

Checklist:

github-actions bot commented Jan 3, 2025

github-actions bot commented Jan 3, 2025

github-actions bot commented Jan 3, 2025

github-actions bot commented Jan 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 7, 2025