feat(platform): Add skeleton loader in secret

[Nirajan1-droid]

User description

Description

Implemented a loading indicator using Loading component in the SecretPage component to enhance user experience during data fetching.
for skeleton i have used : shadcn-ui@latest - shadcn-ui@0.8.0 to generate the skeleton component.
Skeleton component then is used by Loading Component and customized as per design.

Fixes #322

Dependencies

Future Improvements

depends on the test case and feedback.

Mentions

@rajdip-b
@kriptonian1

Screenshots of relevant screens

1.default view.

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, Tests

---

Description

• Integrated Loading component in SecretPage to enhance user experience during data fetching.
• Managed loading state using useState and useEffect hooks.
• Created a new Loading component that displays a skeleton UI.

---

Changes walkthrough 📝

Relevant files

Enhancement

page.tsx Integrate Loading for data fetching in SecretPage  apps/platform/src/app/(main)/project/[project]/@secret/page.tsx Added Loading component to handle loading state. Managed loading state using useState and useEffect. Rendered Loading conditionally based on loading state. +75/-67  Loading .tsx Create Loading component for loading states              apps/platform/src/components/ui/Loading .tsx Created Loading component for displaying loading skeletons. Implemented toggle functionality for accordion-like skeleton rows. Added accessibility attributes and animations for better UX. +152/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-free]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug: The SkeletonLoader component is imported and used in the SecretPage component, but there is no check to ensure that allSecrets is not null before attempting to map over it. This could lead to runtime errors if allSecrets is null. Code Redundancy: The SkeletonLoader component contains repeated code for generating skeleton rows. Consider refactoring this to a more modular approach to enhance maintainability and reduce code duplication.

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use a finally block to ensure the loading state is updated regardless of the request outcome Instead of setting the loading state to false in both the then and catch blocks, you can use a finally block to ensure it is always set to false regardless of whether the request succeeds or fails. apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [41-49] Secrets.getAllSecretbyProjectId(pathname.split('/')[2]) .then((data) => { setAllSecrets(data) - setLoading(false); // loading becomes false once data is fetched }) .catch((error) => { console.error(error) - setLoading(false); // Handling loading state in case of error + }) + .finally(() => { + setLoading(false); // Ensure loading state is updated regardless of outcome }) Apply this suggestion Suggestion importance[1-10]: 10 Why: This suggestion correctly identifies a best practice in handling asynchronous operations, ensuring the loading state is always correctly set, which is crucial for the user experience.	10
Possible issue	Manage the open/close state individually for each row to ensure independent toggling The isOpen state should be managed individually for each row to ensure that toggling one row does not affect the others. apps/platform/src/components/ui/SkeletonLoader.tsx [30-36] -const [isOpen, setIsOpen] = useState(false); // State to manage open/close state -const toggleAccordion = () => { - setIsOpen(!isOpen); +const [openRows, setOpenRows] = useState<number[]>([]); // State to manage open rows +const toggleAccordion = (index: number) => { + setOpenRows((prev) => + prev.includes(index) ? prev.filter((i) => i !== index) : [...prev, index] + ); }; Suggestion importance[1-10]: 9 Why: This suggestion correctly addresses a potential issue where toggling one row affects all others, which is crucial for the component's functionality and user interaction.	9
Maintainability	Extract the logic for rendering the AccordionItem into a separate component for better readability and maintainability To improve readability and maintainability, consider extracting the logic for rendering the AccordionItem into a separate component. apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [64-124] -{allSecrets?.map((secret) => { - return ( - <AccordionItem - className="rounded-xl bg-white/5 px-5" - key={secret.secret.id} - value={secret.secret.id} - > - <AccordionTrigger - className="hover:no-underline" - rightChildren={ - <div className="text-xs text-white/50"> - {dayjs(secret.secret.updatedAt).toNow(true)} ago by{' '} - <span className="text-white"> - {secret.secret.lastUpdatedBy.name} - </span> - </div> - } - > - <div className="flex gap-x-5"> - <div className="flex items-center gap-x-4"> - {secret.secret.name} - </div> - {secret.secret.note ? ( - <TooltipProvider> - <Tooltip> - <TooltipTrigger> - <NoteIconSVG className="w-7" /> - </TooltipTrigger> - <TooltipContent className="border-white/20 bg-white/10 text-white backdrop-blur-xl"> - <p>{secret.secret.note}</p> - </TooltipContent> - </Tooltip> - </TooltipProvider> - ) : null} - </div> - </AccordionTrigger> - <AccordionContent> - <Table> - <TableHeader> - <TableRow> - <TableHead>Environment</TableHead> - <TableHead>Secret</TableHead> - </TableRow> - </TableHeader> - <TableBody> - {secret.values.map((value) => { - return ( - <TableRow key={value.environment.id}> - <TableCell>{value.environment.name}</TableCell> - <TableCell className="max-w-40 overflow-auto"> - {value.value} - </TableCell> - </TableRow> - ) - })} - </TableBody> - </Table> - </AccordionContent> - </AccordionItem> - ) -})} +{allSecrets?.map((secret) => ( + <SecretAccordionItem key={secret.secret.id} secret={secret} /> +))} Apply this suggestion Suggestion importance[1-10]: 8 Why: Extracting the AccordionItem into a separate component enhances code maintainability and readability, which is a significant improvement in a large codebase.	8
Accessibility	Add aria-live="polite" to the container div to improve accessibility for screen readers To improve accessibility, add aria-live="polite" to the container div to announce changes in the loading state to screen readers. apps/platform/src/components/ui/SkeletonLoader.tsx [39] -<div className="grid grid-cols-1 gap-4"> +<div className="grid grid-cols-1 gap-4" aria-live="polite"> Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding aria-live="polite" improves accessibility by announcing dynamic content changes, which is important for users relying on screen readers.	7

[rajdip-b]

@kriptonian1 I feel the color scheme can be tweaked a bit? Perhaps the transparancy.

[kriptonian1]

@kriptonian1 I feel the color scheme can be tweaked a bit? Perhaps the transparancy.

Yeap, the color scheme and the component is very off. Let me make a quick design in figma and share the details here

[Nirajan1-droid]

ok 👍 waiting for that design

[kriptonian1]

@Nirajan1-droid make a small component called loader and just add the style like this, you dosen't need to make dropdown, and use shadcn/ui's loader to make the component. the current implementation is not right.

read this and implement
https://nextjs.org/docs/app/building-your-application/routing/loading-ui-and-streaming

[Nirajan1-droid]

ok working on it.

[Nirajan1-droid]

is it ok? :)

[kriptonian1]

is it ok? :)

the UI looks good to me, let me review the code

[Nirajan1-droid]

waiting for feedback :) 👍

[Nirajan1-droid]

hope i get feedback again :) 👍

[Nirajan1-droid]

@kriptonian1 is there any more feedback for improvements or, is it ready for merge?

[kriptonian1]

@kriptonian1 is there any more feedback for improvements or, is it ready for merge?

Let me go through it

[kriptonian1]

@Nirajan1-droid will it be possible for you to send me a video of the React suspense working, it will really help me to review it

[Nirajan1-droid]

i faced some issue with the usage of suspense, as skeleton didn't appear when i tried fallback to the loading component.
i thought it was because i haven't run backend, but as per the contribution(review) demand, i added it while pushing the changes.
but it did work without suspense.

[kriptonian1]

i faced some issue with the usage of suspense, as skeleton didn't appear when i tried fallback to the loading component. i thought it was because i haven't run backend, but as per the contribution(review) demand, i added it while pushing the changes. but it did work without suspense.

You are supposed to run the backend and then test it out otherwise the implementation can be buggy

[Nirajan1-droid]

previously, the code used loading state through the usage of useState.
that method did work and skeleton appeared at loading time.

[Nirajan1-droid]

yes. but i faced some issues while running it. i tried it with docker also but can't resolve it. so, i got into it without backend.

[kriptonian1]

what's the issue

[Nirajan1-droid]

at first i got:
npm WARN using --force Recommended protections disabled.
npm ERR! code EUNSUPPORTEDPROTOCOL
npm ERR! Unsupported URL Type "workspace:": workspace:*

npm ERR! A complete log of this run can be found in: C:\Users\Nira\AppData\Local\npm-cache_logs\2024-07-06T15_12_01_810Z-debug-0.log

and i tried to resolve with docker:
but again got the same error:

RUN npm install:
67.72 npm notice
67.72 npm notice New minor version of npm available! 10.7.0 -> 10.8.1
67.72 npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.8.1
67.72 npm notice To update run: npm install -g npm@10.8.1
67.72 npm notice
67.73 npm error code EUNSUPPORTEDPROTOCOL
67.73 npm error Unsupported URL Type "workspace:": workspace:*

next i tried with yarn but didn't workout.
then i remove some dependcies related to workspace in package.json like eslint, tsconfig, then only it worked and dependencies got install after so much time wastage in it, and due to this configuration errors , i didn't even tried to run the backend as of now.

[kriptonian1]

why are you using npm, this is a pnpm project. I will recommend for you to read the doc and then start contributing.

https://docs.keyshade.xyz/contributing-to-keyshade/setting-things-up

[kriptonian1]

@Nirajan1-droid figured antything out ?

[Nirajan1-droid]

nope, still stuck in backend configuration.
the login didn't happen although everything worked right, after configuring with pnpm. i did also setup postgresql database.
still trying to figure out.

[kriptonian1]

Bro, read the docs you have to use docker.

[kriptonian1]

any updates?

[kriptonian1]

@Nirajan1-droid any updates ????

[kriptonian1]

@Nirajan1-droid if the issue is done then can you send me a video

[rajdip-b]

@Nirajan1-droid any progress on this yet?

[kriptonian1]

@rajdip-b If there is no significant progress then I think we should close this commit, and let others give it a try

[Nirajan1-droid]

i got into small accident.

…

On Fri, Aug 2, 2024, 7:43 PM Sawan Bhattacharya ***@***.***> wrote: @rajdip-b <https://github.com/rajdip-b> If there is no significant progress then I think we should close this commit, and let others give it a try — Reply to this email directly, view it on GitHub <#328 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APUMBHZJLXFKXUXP7OJSGV3ZPOGARAVCNFSM6AAAAABKOWJJBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRVGQ3DCNRUHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>