fix(web): resolve encryption glitch in footer text

Aleesssino commented 3 weeks ago

User description

Description

I added state management to store the element's width before the encryption animation begins. This ensures that when the encryption starts, the layout remains stable and doesn't break.

Fixes #249

Future Improvements

If others find that this fix doesn't work as expected or if the behavior isn't ideal, I will improve my solution accordingly.

Mentions

@kriptonian1 I am waiting for your feedback.

Screenshots of relevant screens

https://github.com/keyshade-xyz/keyshade/assets/97041873/5c5c0716-cd62-43e8-9a68-1fa0c2236d5e

Developer's checklist

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

If changes are made in the code:

[X] I have followed the coding guidelines
[X] 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
[X] I have added relevant screenshots in my PR
[X] There are no UI/UX issues

PR Type

Bug fix

Description

Added state management to store the element's width before the encryption animation begins to ensure layout stability.
Changed the root element from motion.button to motion.div to better handle the width styling.
Introduced a span with a ref to measure the text width and applied this width to the motion.div.

Changes walkthrough 📝

Relevant files

Bug fix

encrypt-text.tsx

Fix encryption animation glitch by managing element width

apps/web/src/components/ui/encrypt-text.tsx

Added useEffect to store the element's width before the encryption
animation begins.

Changed the root element from motion.button to motion.div.

Added a span with ref to measure the text width.

Applied the measured width to the motion.div to maintain layout
stability.

+19/-8

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

codiumai-pr-agent[bot] commented 3 weeks ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	2
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	None

codiumai-pr-agent[bot] commented 3 weeks ago

PR Code Suggestions ✨

Category	Suggestion	Score
Accessibility	Replace with ___ **To ensure accessibility and semantic correctness, replace the element with a	10
Best practice	Add a cleanup function in `useEffect` to reset the width state on component unmount ___ Consider using a cleanup function in the `useEffect` hook to clear the width state when the component unmounts. This can help prevent potential memory leaks or unexpected behavior when the component is no longer in the DOM but the state updates are triggered. [apps/web/src/components/ui/encrypt-text.tsx [20-24]](https://github.com/keyshade-xyz/keyshade/pull/267/files#diff-0991949f9f3df3982b1910773d024f58ddd46c49ef2121e579fadafaf4839bceR20-R24) ```diff useEffect(() => { if (textRef.current) { setWidth(textRef.current.offsetWidth) } + return () => { + setWidth(null); + }; }, [TARGET_TEXT]) ``` Suggestion importance[1-10]: 9 Why: Adding a cleanup function in the `useEffect` hook is a best practice to prevent potential memory leaks and ensure that the state is reset when the component unmounts. This is a significant improvement for the stability and reliability of the component.	9
Performance	Use `useMemo` for calculating width to optimize performance ___ It is recommended to use `useMemo` for calculating the width based on `textRef.current.offsetWidth` to optimize performance by avoiding unnecessary recalculations during re-renders that do not affect the `textRef.current`. [apps/web/src/components/ui/encrypt-text.tsx [20-24]](https://github.com/keyshade-xyz/keyshade/pull/267/files#diff-0991949f9f3df3982b1910773d024f58ddd46c49ef2121e579fadafaf4839bceR20-R24) ```diff -useEffect(() => { - if (textRef.current) { - setWidth(textRef.current.offsetWidth) - } -}, [TARGET_TEXT]) +const width = useMemo(() => { + return textRef.current ? textRef.current.offsetWidth : null; +}, [textRef.current]); ``` Suggestion importance[1-10]: 7 Why: Using `useMemo` can optimize performance by preventing unnecessary recalculations. However, the current implementation with `useEffect` is also valid and the performance gain might be marginal in this specific context.	7
Maintainability	Use CSS classes instead of inline styles for dynamic width adjustments ___ Avoid using inline styles for setting the width dynamically. Instead, consider using CSS classes and modifying these classes based on the state to make the styles more maintainable and potentially enhance performance by reducing style recalculations. [apps/web/src/components/ui/encrypt-text.tsx [62]](https://github.com/keyshade-xyz/keyshade/pull/267/files#diff-0991949f9f3df3982b1910773d024f58ddd46c49ef2121e579fadafaf4839bceR62-R62) ```diff -style={{ width: width ? `${width}px` : 'auto' }} +className={`group relative w-full overflow-hidden text-white/60 transition-colors hover:text-white ${width ? 'custom-width' : ''}`} ``` Suggestion importance[1-10]: 6 Why: While using CSS classes instead of inline styles can improve maintainability and performance, the current use of inline styles is not a critical issue. The suggestion is valid but addresses a minor improvement.	6

sonarcloud[bot] commented 2 weeks ago

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
1.5% Duplication on New Code

See analysis details on SonarCloud

kriptonian1 commented 2 weeks ago

@rajdip-b it doesn't seem like it's working after merged, can you confirm if it's an issue from the deployment end or not

rajdip-b commented 2 weeks ago

For now, we don't have any staging environment. So, the pipeline is set to work only when code is pushed to main (when a release is made). That's why, it didn't work.

Later on when we will completely deploy the alpha environment, we will set up develop with the staging environment. Hope that clears things out!

kriptonian1 commented 2 weeks ago

Can we merge this pr to prod

keyshade-xyz / keyshade