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

Apply YorkieUI Button component #155

Closed
wants to merge 14 commits into from
Closed

Conversation

chacha912
Copy link
Contributor

@chacha912 chacha912 commented Jul 30, 2024

What this PR does / why we need it?

Apply YorkieUI Button components

  • Replace Button component with Yorkie UI components
  • Replace button element with Yorkie UI components

Future works

  • Replace link style with Yorkie UI
  • Replace toggleButton element with Yorkie UI
  • Replace Icons with Yorkie UI

Any background context you want to provide?

What are the relevant tickets?

Fixes #

Checklist

  • Added relevant tests or not required
  • Didn't break anything

Summary by CodeRabbit

  • New Features

    • Introduced new arrow icons (arrowLeft and arrowRight) to enhance icon options.
    • Updated button functionalities across various components using the new Button from @yorkie-ui/core, improving styling and responsiveness.
  • Improvements

    • Enhanced layout and accessibility in several components by incorporating Flex and Link components for better UI structure.
    • Streamlined theme management for consistent application of light and dark themes.
  • Bug Fixes

    • Removed unused component imports to improve performance and readability.
  • Documentation

    • Updated documentation structure to reflect changes in components and improve clarity.

Copy link

coderabbitai bot commented Jul 30, 2024

Walkthrough

The recent updates focus on integrating the @yorkie-ui/core UI library across various components, enhancing consistency in styling and functionality. Key modifications include replacing standard HTML elements with the new Button component, refining layouts using Flex, and removing deprecated components. These changes improve accessibility, maintainability, and the overall user experience throughout the application.

Changes

Files Change Summary
components/.../CodeBlock.tsx, components/.../CodeBlockHeader.tsx Updated button imports to @yorkie-ui/core, modified properties for CopyButtonBox, and enhanced button functionality for better accessibility.
components/.../Icons/Icon.tsx Added arrowLeft and arrowRight icons with rotation styles to the svgMap, enhancing icon functionality.
components/.../Layout/*.tsx Removed unused imports and centralized button implementations using @yorkie-ui/core, improving the UI and responsiveness.
components/docs/index.ts Removed the export of the Breadcrumb component, altering the public API and simplifying the component set.
components/exampleView/.../*.tsx Replaced standard buttons with Button components from @yorkie-ui/core, enhancing styling and maintaining existing functionality.
hooks/useTheme.ts Introduced handleTheme function for streamlined theme management, improving code clarity and efficiency.
package.json Upgraded the next package version and added @yorkie-ui/core as a new dependency, expanding project capabilities.
pages/.../*.tsx Restructured button implementations across various pages to use Link components wrapped in Button, improving navigation and accessibility.
docs/sample.mdx Removed <Breadcrumb> and simplified button structure, enhancing usability and clarity in documentation.

Poem

🐰 In the meadow where the UI blooms,
Buttons dance, dispelling gloom.
With Yorkie’s charm, we leap and play,
A brighter path lights up the way.
Hopping through changes, swift and spry,
Our interface soars, oh my, oh my! 🌼✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@chacha912 chacha912 marked this pull request as ready for review August 8, 2024 03:10
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a5bee53 and fb2d91e.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (20)
  • components/CodeBlock/CodeBlock.tsx (2 hunks)
  • components/CodeBlock/CodeBlockHeader.tsx (2 hunks)
  • components/Icons/Icon.tsx (1 hunks)
  • components/Layout/ExampleLayout.tsx (1 hunks)
  • components/Layout/Header.tsx (2 hunks)
  • components/Layout/MobileGnbDropdown.tsx (2 hunks)
  • components/docs/index.ts (1 hunks)
  • components/exampleView/BasicView/BasicExampleView.tsx (2 hunks)
  • components/exampleView/Sidebar/Sidebar.tsx (3 hunks)
  • components/index.ts (1 hunks)
  • docs/sample.mdx (2 hunks)
  • hooks/useTheme.ts (1 hunks)
  • package.json (2 hunks)
  • pages/404.tsx (2 hunks)
  • pages/_app.tsx (1 hunks)
  • pages/community.tsx (2 hunks)
  • pages/docs/[[...slug]].tsx (2 hunks)
  • pages/examples/index.tsx (3 hunks)
  • pages/index.tsx (9 hunks)
  • pages/products.tsx (4 hunks)
Files skipped from review due to trivial changes (3)
  • components/Layout/ExampleLayout.tsx
  • docs/sample.mdx
  • pages/_app.tsx
Additional context used
Biome
components/Layout/Header.tsx

[error] 56-56: Avoid redundant double-negation.

It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation

(lint/complexity/noExtraBooleanCast)

Additional comments not posted (32)
components/docs/index.ts (1)

Line range hint 1-7:
Verify the impact of removing Breadcrumb.

The removal of the Breadcrumb export might affect modules that depend on it. Ensure that any dependent code is updated to handle this change.

Verification successful

No impact from removing Breadcrumb export.

The search for the Breadcrumb component across the codebase returned no results, indicating that it is not used elsewhere. Therefore, removing its export will not affect any dependent modules.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the `Breadcrumb` component is used elsewhere in the codebase.

# Test: Search for any usage of `Breadcrumb`. Expect: No occurrences.
rg --type ts --type tsx 'Breadcrumb'

Length of output: 64


Script:

#!/bin/bash
# Description: Verify if the `Breadcrumb` component is used elsewhere in the codebase.

# Test: Search for any usage of `Breadcrumb` in TypeScript and TSX files.
rg --type-add 'tsx:*.tsx' --type ts --type tsx 'Breadcrumb'

Length of output: 59

components/index.ts (1)

Line range hint 1-9:
Verify the impact of removing Button.

The removal of the Button export may affect modules that import it through this index file. Ensure that any dependent code is updated to import Button directly from its source.

Verification successful

Direct imports of Button found in the codebase

The Button component is directly imported from @yorkie-ui/core in multiple locations within the codebase. This indicates that the removal of Button from the components/index.ts file does not impact these parts of the codebase since they do not rely on the index file for importing Button.

  • Direct imports of Button from @yorkie-ui/core were found, suggesting that the removal from the index file is unlikely to cause issues.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the `Button` component is used elsewhere in the codebase.

# Test: Search for any usage of `Button`. Expect: No occurrences or direct imports from its source.
rg --type ts --type tsx 'Button'

Length of output: 60


Script:

#!/bin/bash
# Description: Verify if the `Button` component is used elsewhere in the codebase.

# Test: Search for any usage of `Button`. Expect: No occurrences or direct imports from its source.
rg -t ts -t tsx 'Button'

Length of output: 52


Script:

#!/bin/bash
# Description: Verify if the `Button` component is used elsewhere in the codebase.

# Find all `.ts` and `.tsx` files and search for `Button` within them.
fd -e ts -e tsx -x rg 'Button' {}

Length of output: 23732

hooks/useTheme.ts (1)

5-11: LGTM! Verify consistent usage of handleTheme.

The introduction of handleTheme centralizes theme logic, improving maintainability. Ensure that this function is used consistently across the codebase.

Verification successful

Consistent Usage of handleTheme Verified

The handleTheme function is used consistently within the hooks/useTheme.ts file, aligning with theme-related logic. No inconsistencies were found in its usage.

  • hooks/useTheme.ts: Multiple consistent usages of handleTheme.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of `handleTheme`.

# Test: Search for any usage of `handleTheme`. Expect: Consistent usage within theme-related logic.
rg --type ts --type tsx 'handleTheme'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify consistent usage of `handleTheme`.

# Test: Search for any usage of `handleTheme`. Expect: Consistent usage within theme-related logic.
rg 'handleTheme' --glob '*.ts' --glob '*.tsx'

Length of output: 352

components/CodeBlock/CodeBlockHeader.tsx (3)

2-3: Import change approved.

The import of the Button component from @yorkie-ui/core aligns with the PR's objective to standardize UI components.


31-33: Button component usage approved.

The use of variant, size, and colorPalette properties aligns with the new UI framework, enhancing button appearance and behavior.


17-25: Function signature update approved.

The addition of the size parameter enhances flexibility and adaptability of the CopyButtonBox.

Ensure that the usage of this parameter is consistent across the codebase.

package.json (2)

45-45: Addition of @yorkie-ui/core approved.

The addition aligns with the PR's objective to integrate YorkieUI components.


24-24: Next.js version update approved.

The update from 13.0.0 to 13.4.7 may bring performance improvements and new features.

Ensure compatibility with the existing codebase and test thoroughly.

components/CodeBlock/CodeBlock.tsx (2)

2-3: Import change approved.

The import of the Button component from @yorkie-ui/core aligns with the PR's objective to standardize UI components.


31-33: Button component usage approved.

The use of size, variant, and colorPalette properties aligns with the new UI framework, enhancing button appearance and behavior.

pages/community.tsx (1)

30-43: LGTM!

The use of Flex and Button components enhances layout flexibility and accessibility. The asChild prop effectively integrates Link components, aligning with the PR objectives.

pages/404.tsx (1)

27-46: LGTM!

The use of Flex and Button components enhances layout flexibility and accessibility. The asChild prop effectively integrates Link components, aligning with the PR objectives.

components/exampleView/Sidebar/Sidebar.tsx (2)

40-52: LGTM!

The use of the Button component for toggling the sidebar enhances visual consistency and improves state management. The conditional icon display is a user-friendly touch.


68-79: LGTM!

The use of Button components wrapped around Link components enhances semantic structure and accessibility. The layout improvements align with the PR objectives.

components/Icons/Icon.tsx (1)

64-65: Ensure consistent styling for rotated icons.

The style prop is used to rotate the ArrowSVG component. Verify that this approach aligns with the rest of the codebase's styling practices.

components/exampleView/BasicView/BasicExampleView.tsx (2)

103-113: Ensure Button component props are used effectively.

The Button component is configured with variant, size, and colorPalette props. Ensure these props align with the intended design and functionality.


118-120: Check positioning props for layout consistency.

The Button for adding a user uses positioning props. Verify that these props align with the layout and design standards.

pages/docs/[[...slug]].tsx (2)

23-23: Import change approved.

The Button component is now imported from @yorkie-ui/core, aligning with the objective of standardizing button styles.


22-24: Verify the removal of Breadcrumb.

Ensure that the removal of the Breadcrumb component does not impact the document's functionality.

Verification successful

Breadcrumb removal confirmed as non-impactful.

The Breadcrumb component is not used in any .tsx files, indicating its removal does not affect the document's functionality.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `Breadcrumb` removal does not affect functionality.

# Test: Search for `Breadcrumb` usage in the repository. Expect: No usage.
rg --type tsx 'Breadcrumb'

Length of output: 54


Script:

#!/bin/bash
# Description: Correctly verify that `Breadcrumb` removal does not affect functionality.

# Find all .tsx files and search for `Breadcrumb` usage within them.
fd --extension tsx --exec rg 'Breadcrumb' {}

Length of output: 46

components/Layout/MobileGnbDropdown.tsx (2)

6-6: Import change approved.

The Button component is now imported from @yorkie-ui/core, aligning with the objective of standardizing button styles.


26-29: Button change approved.

The change from <button> to the Button component enhances UI consistency and styling.

pages/examples/index.tsx (2)

7-7: Import change approved.

The Button component is now imported from @yorkie-ui/core, aligning with the objective of standardizing button styles.


25-30: Button usage change approved.

The updated Button usage improves appearance, responsiveness, and navigation experience.

pages/products.tsx (3)

6-6: Import change approved.

The Button component is correctly imported from @yorkie-ui/core, aligning with the PR objective of integrating YorkieUI components.


259-272: Button component styling approved.

The Button component is used with appropriate styling props and correctly wraps a Link for navigation.

Verify that the styling props are correctly applied and consistent with design guidelines.

Verification successful

Button component styling verified and consistent.

The styling props applied to the Button components are used consistently and appropriately throughout the codebase, aligning with the intended design and layout requirements. This confirms the initial approval of the styling changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of styling props applied to Button components.

# Test: Search for Button components with styling props. Expect: Consistent and correct usage.
rg --type jsx -A 5 $'<Button.*(size|mt|width|colorPalette|position|top|right)'

Length of output: 107


Script:

#!/bin/bash
# Description: Verify the correctness of styling props applied to Button components.

# Find all .tsx files and search for Button components with specific styling props.
fd --extension tsx --exec rg -A 5 '<Button.*(size|mt|width|colorPalette|position|top|right)' {}

Length of output: 6871


37-42: Button component usage approved.

The Button component correctly uses the asChild prop to wrap a Link, enhancing semantic clarity and accessibility.

Verify that the asChild prop is used consistently across the codebase.

Verification successful

Consistent usage of the asChild prop in Button components confirmed.

The asChild prop is used consistently across various files to wrap Link components within Button components, ensuring semantic clarity and accessibility.

  • pages/products.tsx
  • pages/index.tsx
  • pages/examples/index.tsx
  • pages/community.tsx
  • pages/404.tsx
  • components/exampleView/Sidebar/Sidebar.tsx
  • components/Layout/Header.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of the `asChild` prop in Button components.

# Test: Search for Button components using the `asChild` prop. Expect: Consistent usage.
rg --type jsx -A 2 $'<Button asChild'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify consistent usage of the `asChild` prop in Button components.

# Test: Search for Button components using the `asChild` prop. Expect: Consistent usage.
rg --type js -A 2 $'<Button asChild'

Length of output: 37


Script:

#!/bin/bash
# Description: Broaden search for the `asChild` prop usage in Button components across all file types.

# Test: Search for Button components using the `asChild` prop across all files. Expect: Consistent usage.
rg -A 2 $'<Button asChild'

Length of output: 5551

pages/index.tsx (6)

6-6: Import change approved.

The Button and Flex components are correctly imported from @yorkie-ui/core, aligning with the PR objective of integrating YorkieUI components.


60-69: Button component usage approved.

The Button component correctly uses the asChild prop to wrap a Link, enhancing semantic clarity and accessibility.

Verify that the asChild prop is used consistently across the codebase.


242-247: Button component usage approved.

The Button component uses the asChild prop to wrap a Link, ensuring correct navigation and styling.

Verify that the asChild prop is used consistently across the codebase.


377-397: Button component usage approved.

The Button component uses the asChild prop to wrap a Link, ensuring correct navigation and styling.

Verify that the asChild prop is used consistently across the codebase.


219-219: CopyButton size prop usage approved.

The CopyButton component includes a size prop, enhancing usability and ensuring design consistency.

Verify that the size prop is used consistently in CopyButton components across the codebase.

Verification successful

Single Usage of CopyButton with size Prop Verified

The size prop in the CopyButton component is used correctly in pages/index.tsx. However, it is the only instance found in the codebase, so consistency across multiple uses cannot be verified.

  • File: pages/index.tsx
  • Line: 219
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `size` prop in CopyButton components.

# Test: Search for CopyButton components with the `size` prop. Expect: Consistent usage.
rg --type jsx -A 2 $'<CodeBlockHeader.CopyButton.*size'

Length of output: 84


Script:

#!/bin/bash
# Description: Verify the usage of the `size` prop in CopyButton components.

# Test: Search for CopyButton components with the `size` prop. Expect: Consistent usage.
rg --type tsx -A 2 $'<CodeBlockHeader.CopyButton.*size'

Length of output: 84


Script:

#!/bin/bash
# Description: Verify the usage of the `size` prop in CopyButton components.

# Test: Search for CopyButton components with the `size` prop in all files.
rg -A 2 $'<CodeBlockHeader.CopyButton.*size'

Length of output: 265


93-113: Flex component usage approved.

The Flex component is effectively used for layout control, providing flexible spacing and alignment for Button components.

Verify that the Flex component is used consistently for layout purposes across the codebase.

Verification successful

Consistent usage of the Flex component for layout purposes verified.

The Flex component is effectively and consistently used for layout management across multiple files in the codebase, ensuring a standardized approach to spacing and alignment.

  • pages/index.tsx
  • pages/community.tsx
  • pages/404.tsx
  • components/Layout/Header.tsx
  • components/exampleView/Sidebar/Sidebar.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of the `Flex` component for layout purposes.

# Test: Search for Flex component usage. Expect: Consistent and effective usage for layout.
rg --type jsx -A 5 $'<Flex'

Length of output: 56


Script:

#!/bin/bash
# Description: Verify consistent usage of the `Flex` component for layout purposes.

# Test: Search for Flex component usage in TSX files. Expect: Consistent and effective usage for layout.
rg --type tsx -A 5 $'<Flex'

Length of output: 56


Script:

#!/bin/bash
# Description: Verify consistent usage of the `Flex` component for layout purposes.

# Test: Search for Flex component usage across all files. Expect: Consistent and effective usage for layout.
rg -A 5 '<Flex'

Length of output: 3128

Comment on lines +55 to +73
<Flex gap="200" hideBelow="md">
{!!isLoggedIn ? (
<Button asChild variant="outline" colorPalette="neutral">
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}`}>Dashboard</Link>
</Button>
<Button
as="a"
href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/signup`}
className="orange_0"
icon={<Icon type="star" />}
>
Start for free
</Button>
</>
) : null}
) : (
<>
<Button asChild variant="outline" colorPalette="neutral">
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/login`}>Sign in</Link>
</Button>
<Button asChild>
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/signup`}>
<Icon type="star" />
Start for free
</Link>
</Button>
</>
)}
</Flex>
Copy link

Choose a reason for hiding this comment

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

Use direct boolean coercion instead of double-negation.

The double-negation !!isLoggedIn is unnecessary as isLoggedIn is already a boolean.

- {!!isLoggedIn ? (
+ {isLoggedIn ? (
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Flex gap="200" hideBelow="md">
{!!isLoggedIn ? (
<Button asChild variant="outline" colorPalette="neutral">
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}`}>Dashboard</Link>
</Button>
<Button
as="a"
href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/signup`}
className="orange_0"
icon={<Icon type="star" />}
>
Start for free
</Button>
</>
) : null}
) : (
<>
<Button asChild variant="outline" colorPalette="neutral">
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/login`}>Sign in</Link>
</Button>
<Button asChild>
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/signup`}>
<Icon type="star" />
Start for free
</Link>
</Button>
</>
)}
</Flex>
<Flex gap="200" hideBelow="md">
{isLoggedIn ? (
<Button asChild variant="outline" colorPalette="neutral">
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}`}>Dashboard</Link>
</Button>
) : (
<>
<Button asChild variant="outline" colorPalette="neutral">
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/login`}>Sign in</Link>
</Button>
<Button asChild>
<Link href={`${process.env.NEXT_PUBLIC_DASHBOARD_PATH}/signup`}>
<Icon type="star" />
Start for free
</Link>
</Button>
</>
)}
</Flex>
Tools
Biome

[error] 56-56: Avoid redundant double-negation.

It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation

(lint/complexity/noExtraBooleanCast)

@hackerwins
Copy link
Member

For now, it seems more realistic to organize the existing SaaS-based UI rather than apply the new Yorkie UI. I will close this issue.

@hackerwins hackerwins closed this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants