feat: Added Pagination to Environments Screen #780

chetan102 · 2025-02-22T11:45:49Z

User description

Description

Now we dont get the all environments at one , now we can get it in the batch of 10 if user wants to see the next 10 environments they have to click on Load More Button

Fixes #707

Screenshots of relevant screens

Developer's checklist

My PR follows the style guidelines of this project
I have performed a self-check on my work

If changes are made in the code:

I have followed the coding guidelines
My changes in code generate no new warnings
My changes are breaking another fix/feature of the project
I have added test cases to show that my feature works
I have added relevant screenshots in my PR
There are no UI/UX issues

Documentation Update

This PR requires an update to the documentation at docs.keyshade.xyz
I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Enhancement

Description

Added dynamic pagination to the environments page.
Introduced a "Load More" button for additional environments.
Limited initial environment display to 10 items.
Updated UI layout to support dynamic loading.

Changes walkthrough 📝

Relevant files

Enhancement

page.tsx `Added dynamic pagination and "Load More" button` apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx Added state management for visible environment count. Limited initial environment display to 10 items. Added "Load More" button to load additional environments. Updated UI structure to support dynamic pagination.	+26/-16

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

codiumai-pr-agent-free · 2025-02-22T11:46:11Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 707 - PR Code Verified Compliant requirements: Show first 10 environments by default Add "Load More" button at end of page Load More button shows next 10 environments on click Add dynamic pagination on environments page Requires further human verification: Verify UI/UX matches Figma design Test pagination behavior with 20+ environments
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Performance The slice operation on environments array is performed on every render. Consider memoizing the sliced array using useMemo to optimize performance. {environments.slice(0, visibleCount).map((environment) => ( Code Style The onClick handler for Load More button uses an inline arrow function. Consider extracting this to a named function for better readability and reusability. <Button onClick={()=> setVisibleCount((prev) => prev + INITIAL_DISPLAY_COUNT)}>Load More</Button>

codiumai-pr-agent-free · 2025-02-22T11:46:26Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ ~~Prevent concurrent loading operations~~ Suggestion Impact: The commit implemented loading state handling for the Load More button, including disabling it during loading and showing loading text, though with a different implementation approach using pagination code diff: + <Button disabled={loading} onClick={handlePageShift}> + {loading ? 'Loading...' : 'Load More'} + </Button> Add loading state to prevent multiple "Load More" clicks while data is being fetched, which could cause race conditions or duplicate loads. apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx [96] -<Button onClick={()=> setVisibleCount((prev) => prev + INITIAL_DISPLAY_COUNT)}>Load More</Button> +<Button + onClick={()=> setVisibleCount((prev) => prev + INITIAL_DISPLAY_COUNT)} + disabled={isLoading} +> + {isLoading ? 'Loading...' : 'Load More'} +</Button> `[Suggestion has been applied]` Suggestion importance[1-10]: 8 __ Why: Adding loading state handling is crucial for preventing race conditions and improving UX by disabling the button during data fetches. This helps avoid potential bugs from multiple concurrent requests.	Medium
Possible issue	✅ ~~Handle potential undefined values~~ Suggestion Impact: Added optional chaining operator to safely handle potential undefined values when accessing environments array code diff: - {environments.slice(0, visibleCount).map((environment) => ( + {environments?.slice(0, visibleCount)?.map((environment) => ( Add error handling for cases where the environment data slice operation fails or returns undefined results. apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx [80-82] -{environments.slice(0, visibleCount).map((environment) => ( - <EnvironmentCard environment={environment} key={environment.id} /> +{environments?.slice(0, visibleCount)?.map((environment) => ( + environment && <EnvironmentCard environment={environment} key={environment.id} /> ))} `[Suggestion has been applied]` Suggestion importance[1-10]: 6 __ Why: Adding null checks improves code robustness by safely handling potential undefined values, though the current context suggests environments is already defined as an array through the useAtom hook.	Low
Update

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

chetan102 · 2025-02-22T12:08:00Z

@rajdip-b, Oh i see is it like increasing the perPage when the User clicks the Load More. like preview is 10 then again +10 = 20 when clicking on the Load More , and while api call it still holds the previous environments value before showing the new 20 environment is it like this is it like this.

rajdip-b · 2025-02-22T12:14:15Z

yeap

chetan102 · 2025-02-23T09:40:44Z

@rajdip-b Now i done changes according to it u can check it.

rajdip-b · 2025-02-23T09:51:08Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

-  const [visibleCount, setVisibleCount] = useState<number>(INITIAL_DISPLAY_COUNT)
+  const [page, setPage] = useState(0)
+  const [hasMore, setHasMore] = useState(true)
+  const [loading, setLoading] = useState(false)


rename this to isLoading to match out conventions

rajdip-b · 2025-02-23T09:53:13Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

-          setEnvironments(data.items)
+          const newData = page === 0 ? data.items : [...environments, ...data.items];
+          setEnvironments(newData);
+          if (newData.length >= data.metadata.totalCount) setHasMore(false);


Instead of this complex calculation, you can check for thw metadata.next field in the response. It will be null if this is the last page.

i didnt find any next key

rajdip-b · 2025-02-23T09:55:02Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

+      })
+  }, [selectedProject, page, getAllEnvironmentsOfProject, setEnvironments])
+
+  const handlePageShift = () => {


I feel you should extract all of the logic in the use effect hook to a separate function.

call that function once in use effect

call that function whenever load more is clicked

github-actions · 2025-02-24T12:36:59Z

@chetan102, please resolve all open reviews!

rajdip-b · 2025-02-25T05:01:07Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

@@ -39,24 +39,32 @@ function EnvironmentPage(): React.JSX.Element {
  )

  useEffect(() => {
-    if (!selectedProject) return 
-    setLoading(true)
+  if(!selectedProject) return


You can remove this check since its already present in handleEnvironmentsFetch

rajdip-b · 2025-02-25T05:01:16Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

+   handleEnvironmentsFetch();
+  }, [selectedProject, getAllEnvironmentsOfProject, setEnvironments])
+
+  const handleEnvironmentsFetch = (newPage = 0) => {


rename this to fetchEnvironments

rajdip-b · 2025-02-25T05:01:40Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

+   handleEnvironmentsFetch();
+  }, [selectedProject, getAllEnvironmentsOfProject, setEnvironments])
+
+  const handleEnvironmentsFetch = (newPage = 0) => {


remove newPage. it should be using the page state var

rajdip-b · 2025-02-25T05:03:07Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

-          setEnvironments(newData);
-          if (newData.length >= data.metadata.totalCount) setHasMore(false);
+          const newData = newPage === 0 ? data.items : [...environments, ...data.items]
+          if(newPage == 0 && page !== 0) setPage(0);


this line doesnt make sense. initially you should set page to -1 or null. before calling getAllEnvironmentsOfProject, you should always increment the value of page using setPage(prev => prev+1)

rajdip-b · 2025-02-25T05:03:38Z

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx

+          const newData = newPage === 0 ? data.items : [...environments, ...data.items]
+          if(newPage == 0 && page !== 0) setPage(0);
+          setEnvironments(newData)
+          if (newData.length >= data.metadata.totalCount) setHasMore(false)


you can check for data.metadata.next. if its null, you can set hasMore to false

chetan102 added 2 commits February 22, 2025 17:03

completed the dynamic pagination for environments

fd1b758

fixed the count value

2fd7286

chetan102 requested a review from kriptonian1 as a code owner February 22, 2025 11:45

codiumai-pr-agent-free bot added the Review effort 2/5 label Feb 22, 2025

rajdip-b requested changes Feb 22, 2025

View reviewed changes

apps/platform/src/app/(main)/(project)/[workspace]/[project]/@environment/page.tsx Show resolved Hide resolved

handled potential environments undefined values

49f5e29

improved pagination feature

3b8c6cc

rajdip-b requested changes Feb 23, 2025

View reviewed changes

rajdip-b mentioned this pull request Feb 23, 2025

feat(platform): added pagination & loading state for variables page #787

Open

8 tasks

improved pagination

184c710

chetan102 requested a review from rajdip-b February 24, 2025 18:23

rajdip-b requested changes Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added Pagination to Environments Screen #780

feat: Added Pagination to Environments Screen #780

chetan102 commented Feb 22, 2025 •

edited

Loading

codiumai-pr-agent-free bot commented Feb 22, 2025

codiumai-pr-agent-free bot commented Feb 22, 2025 •

edited

Loading

chetan102 commented Feb 22, 2025 •

edited

Loading

rajdip-b commented Feb 22, 2025

chetan102 commented Feb 23, 2025

rajdip-b Feb 23, 2025

rajdip-b Feb 23, 2025

chetan102 Feb 24, 2025

rajdip-b Feb 23, 2025

github-actions bot commented Feb 24, 2025

rajdip-b Feb 25, 2025

rajdip-b Feb 25, 2025

rajdip-b Feb 25, 2025

rajdip-b Feb 25, 2025

rajdip-b Feb 25, 2025

feat: Added Pagination to Environments Screen #780

Are you sure you want to change the base?

feat: Added Pagination to Environments Screen #780

Conversation

chetan102 commented Feb 22, 2025 • edited Loading

User description

Description

Screenshots of relevant screens

Developer's checklist

Documentation Update

PR Type

Description

Changes walkthrough 📝

codiumai-pr-agent-free bot commented Feb 22, 2025

PR Reviewer Guide 🔍

codiumai-pr-agent-free bot commented Feb 22, 2025 • edited Loading

PR Code Suggestions ✨

chetan102 commented Feb 22, 2025 • edited Loading

rajdip-b commented Feb 22, 2025

chetan102 commented Feb 23, 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

github-actions bot commented Feb 24, 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

chetan102 commented Feb 22, 2025 •

edited

Loading

codiumai-pr-agent-free bot commented Feb 22, 2025 •

edited

Loading

chetan102 commented Feb 22, 2025 •

edited

Loading