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

feat(platform): Add loading skeleton in the secrets page #423

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

PriyobrotoKar
Copy link
Contributor

@PriyobrotoKar PriyobrotoKar commented Sep 12, 2024

User description

Description

Implemented the shadcn/ui skeleton to render a loader while fetching secrets from the server in the secrets page.

Fixes #322

Dependencies

shadcn/ui-Skeleton

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Screen.Recording.2024-09-12.at.11.32.44.PM.mov

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

  • Introduced a loading skeleton on the secrets page to improve user experience while data is being fetched.
  • Created a new Skeleton component for consistent loading animations across the application.
  • Managed loading state using a boolean isLoading and displayed skeleton loaders conditionally.
  • Enhanced the UI by providing visual feedback during data fetching operations.

Changes walkthrough 📝

Relevant files
Enhancement
page.tsx
Implement loading skeleton for secrets page                           

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

  • Added a loading state with isLoading variable.
  • Implemented SerectLoader component to display skeleton loaders.
  • Integrated Skeleton component for loading visuals.
  • Managed loading state with useEffect and finally block.
  • +34/-0   
    skeleton.tsx
    Add reusable Skeleton component for loading states             

    apps/platform/src/components/ui/skeleton.tsx

  • Created Skeleton component for loading animations.
  • Utilized animate-pulse for visual effect.
  • Allowed custom class names via props.
  • +18/-0   

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

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Error Handling
    The error handling in the useEffect hook only logs the error to the console. Consider implementing a more robust error handling mechanism, such as displaying an error message to the user.

    Code Duplication
    The SerectLoader component is rendered three times with identical props. Consider using a loop or mapping function to reduce code duplication.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Extract loading state logic into a custom hook for better organization

    Consider extracting the loading state logic into a custom hook, such as useSecrets,
    to improve code organization and reusability.

    apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [36-53]

    -const [allSecrets, setAllSecrets] = useState<Secret[]>()
    -const [isLoading, setIsLoading] = useState<boolean>(true)
    -const pathname = usePathname()
    +const { allSecrets, isLoading } = useSecrets(pathname.split('/')[2])
     
    -useEffect(() => {
    -  setIsLoading(true)
    -  Secrets.getAllSecretbyProjectId(pathname.split('/')[2])
    -    .then((data) => {
    -      setAllSecrets(data)
    -    })
    -    .catch((error) => {
    -      // eslint-disable-next-line no-console -- we need to log the error
    -      console.error(error)
    -    })
    -    .finally(() => {
    -      setIsLoading(false)
    -    })
    -}, [pathname])
    +// ... rest of the component code
     
    +// In a separate file or at the top of the current file:
    +function useSecrets(projectId: string) {
    +  const [allSecrets, setAllSecrets] = useState<Secret[]>()
    +  const [isLoading, setIsLoading] = useState<boolean>(true)
    +
    +  useEffect(() => {
    +    setIsLoading(true)
    +    Secrets.getAllSecretbyProjectId(projectId)
    +      .then((data) => {
    +        setAllSecrets(data)
    +      })
    +      .catch((error) => {
    +        console.error(error)
    +      })
    +      .finally(() => {
    +        setIsLoading(false)
    +      })
    +  }, [projectId])
    +
    +  return { allSecrets, isLoading }
    +}
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Extracting the loading logic into a custom hook enhances code organization and reusability, making it easier to manage and test the loading state logic separately.

    8
    Add a size prop to the Skeleton component for easier customization

    Consider adding a size prop to the Skeleton component to allow for easy
    customization of width and height, reducing the need for custom class names for
    common use cases.

    apps/platform/src/components/ui/skeleton.tsx [3-16]

    +interface SkeletonProps extends React.HTMLAttributes<HTMLDivElement> {
    +  size?: 'sm' | 'md' | 'lg';
    +}
    +
     function Skeleton({
       className,
    +  size = 'md',
       ...props
    -}: React.HTMLAttributes<HTMLDivElement>): React.JSX.Element {
    +}: SkeletonProps): React.JSX.Element {
    +  const sizeClasses = {
    +    sm: 'h-4 w-16',
    +    md: 'h-6 w-24',
    +    lg: 'h-8 w-32',
    +  };
    +
       return (
         <div
           className={cn(
             'animate-pulse rounded-md bg-white/20 dark:bg-zinc-800',
    +        sizeClasses[size],
             className
           )}
           {...props}
         />
       )
     }
     
    Suggestion importance[1-10]: 7

    Why: Adding a size prop enhances the flexibility and usability of the Skeleton component, allowing for easier customization and reducing the need for repetitive custom class names.

    7
    Maintainability
    Rename the loading component to better reflect its purpose

    Consider using a more descriptive name for the SerectLoader component, such as
    SecretSkeletonLoader or SecretLoadingPlaceholder, to better reflect its purpose and
    avoid potential confusion with the word "Secret".

    apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [139-155]

    -function SerectLoader(): React.JSX.Element {
    +function SecretSkeletonLoader(): React.JSX.Element {
       return (
         <div className=" rounded-xl bg-white/5 p-4">
           <div className="flex justify-between">
             <div className="flex items-center gap-x-6">
               <Skeleton className=" h-6 w-32 rounded" />
               <Skeleton className=" size-6 rounded" />
             </div>
             <div className="flex items-center gap-x-3">
               <Skeleton className=" h-6 w-24 rounded" />
               <Skeleton className=" h-6 w-16 rounded" />
               <Skeleton className=" ml-5 size-4 rounded" />
             </div>
           </div>
         </div>
       )
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves code readability and maintainability by using a more descriptive name, which helps avoid confusion and better conveys the component's purpose.

    7
    Use a constant for the number of skeleton loaders to improve flexibility

    Consider using a constant for the number of skeleton loaders to render, making it
    easier to adjust the number of placeholders in the future.

    apps/platform/src/app/(main)/project/[project]/@secret/page.tsx [55-63]

    +const SKELETON_COUNT = 3;
    +
     if (isLoading) {
       return (
         <div className="space-y-4">
    -      <SerectLoader />
    -      <SerectLoader />
    -      <SerectLoader />
    +      {Array.from({ length: SKELETON_COUNT }).map((_, index) => (
    +        <SerectLoader key={index} />
    +      ))}
         </div>
       )
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Using a constant for the number of skeleton loaders improves maintainability by making it easier to adjust the number of loaders without modifying multiple lines of code.

    6

    Copy link
    Contributor

    @kriptonian1 kriptonian1 left a comment

    Choose a reason for hiding this comment

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

    LGTM 🚀

    @kriptonian1
    Copy link
    Contributor

    @rajdip-b you can merge this PR

    @rajdip-b rajdip-b merged commit a97681e into keyshade-xyz:develop Sep 13, 2024
    6 checks passed
    rajdip-b pushed a commit that referenced this pull request Sep 16, 2024
    ## [2.5.0](v2.4.0...v2.5.0) (2024-09-16)
    
    ### 🚀 Features
    
    * **api-client:** Added workspace controller ([#427](#427)) ([2f4edec](2f4edec))
    * **api-client:** Added workspace role controller ([#430](#430)) ([b03ce8e](b03ce8e))
    * **api-client:** Synced with latest API ([27f4309](27f4309))
    * **api:** Add slug in entities ([#415](#415)) ([89e2fcc](89e2fcc))
    * **api:** Included default workspace details in getSelf function ([#414](#414)) ([e67bbd6](e67bbd6))
    * **platform:** Add loading skeleton in the [secure]s page ([#423](#423)) ([a97681e](a97681e))
    * **schema:** Added a schema package ([01ea232](01ea232))
    * **web:** Update about and careers page ([e167f53](e167f53))
    
    ### 🐛 Bug Fixes
    
    * **api:** Error messages fixed in api-key service ([#418](#418)) ([edfbce0](edfbce0))
    
    ### 📚 Documentation
    
    * Fixed minor typo in postman workspace link ([#411](#411)) ([ed23116](ed23116))
    * Updated Postman links ([444bfb1](444bfb1))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Suppressed version check test in [secure] ([4688e8c](4688e8c))
    * **api:** Update slug generation method ([#420](#420)) ([1f864df](1f864df))
    
    ### 🔨 Code Refactoring
    
    * **API:** Refactor workspace-membership into a separate module ([#421](#421)) ([574170f](574170f))
    * **platform:** added optional chaining due to strict null check ([#413](#413)) ([907e369](907e369))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.5.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    ## [2.5.0](keyshade-xyz/keyshade@v2.4.0...v2.5.0) (2024-09-16)
    
    ### 🚀 Features
    
    * **api-client:** Added workspace controller ([keyshade-xyz#427](keyshade-xyz#427)) ([2f4edec](keyshade-xyz@2f4edec))
    * **api-client:** Added workspace role controller ([keyshade-xyz#430](keyshade-xyz#430)) ([b03ce8e](keyshade-xyz@b03ce8e))
    * **api-client:** Synced with latest API ([27f4309](keyshade-xyz@27f4309))
    * **api:** Add slug in entities ([keyshade-xyz#415](keyshade-xyz#415)) ([89e2fcc](keyshade-xyz@89e2fcc))
    * **api:** Included default workspace details in getSelf function ([keyshade-xyz#414](keyshade-xyz#414)) ([e67bbd6](keyshade-xyz@e67bbd6))
    * **platform:** Add loading skeleton in the [secure]s page ([keyshade-xyz#423](keyshade-xyz#423)) ([a97681e](keyshade-xyz@a97681e))
    * **schema:** Added a schema package ([01ea232](keyshade-xyz@01ea232))
    * **web:** Update about and careers page ([e167f53](keyshade-xyz@e167f53))
    
    ### 🐛 Bug Fixes
    
    * **api:** Error messages fixed in api-key service ([keyshade-xyz#418](keyshade-xyz#418)) ([edfbce0](keyshade-xyz@edfbce0))
    
    ### 📚 Documentation
    
    * Fixed minor typo in postman workspace link ([keyshade-xyz#411](keyshade-xyz#411)) ([ed23116](keyshade-xyz@ed23116))
    * Updated Postman links ([444bfb1](keyshade-xyz@444bfb1))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Suppressed version check test in [secure] ([4688e8c](keyshade-xyz@4688e8c))
    * **api:** Update slug generation method ([keyshade-xyz#420](keyshade-xyz#420)) ([1f864df](keyshade-xyz@1f864df))
    
    ### 🔨 Code Refactoring
    
    * **API:** Refactor workspace-membership into a separate module ([keyshade-xyz#421](keyshade-xyz#421)) ([574170f](keyshade-xyz@574170f))
    * **platform:** added optional chaining due to strict null check ([keyshade-xyz#413](keyshade-xyz#413)) ([907e369](keyshade-xyz@907e369))
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    PLATFORM: Add skeleton loader in secret
    3 participants