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

fix(alert): [alert] updata alert xdesign #2083

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

wuyiping0628
Copy link
Collaborator

@wuyiping0628 wuyiping0628 commented Sep 9, 2024

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • New Features

    • Introduced a new <tiny-tag> component with an icon for file upload, enhancing visual representation and functionality.
    • Added specific styling for the new tag component to improve aesthetics.
    • Expanded styling options by adding a new CSS variable for disabled tags.
  • Bug Fixes

    • Removed the info icon from the alert component, streamlining available icons.
  • Style

    • Updated the primary background color for tags in the theme, affecting the visual presentation.
    • Enhanced maintainability of the disabled tag styling by using a CSS variable.

@wuyiping0628 wuyiping0628 added the documentation Improvements or additions to documentation label Sep 9, 2024
Copy link

coderabbitai bot commented Sep 9, 2024

Walkthrough

This pull request introduces a new <tiny-tag> component in two Vue files, enhancing the visual representation with a success type and an icon for file uploads. It also modifies the index.ts file by removing the info icon from the alert component and updates the primary background color in the theme configuration for tags. Additionally, a new CSS variable for the disabled tag background color is introduced, collectively improving the functionality and aesthetics of the user interface components.

Changes

Files Change Summary
.../tag/slot-default-composition-api.vue, .../tag/slot-default.vue Added <tiny-tag> components with a success type and a class tiny-only-iocn, along with new CSS for styling.
.../alert/index.ts Removed the info icon from the exported icons in the alert component.
.../tag/smb-theme.js, .../tag/vars.less Modified the primary background color for tags and added a new CSS variable for the disabled tag background color.

Possibly related PRs

Suggested labels

developing, bug

Poem

🐰 In the garden where tags bloom bright,
A tiny tag shines with pure delight.
With an icon to share, it spreads the cheer,
A success so sweet, we hold it dear.
So hop along, let’s celebrate this change,
In our code’s meadow, it’s time to rearrange! 🌼✨

Tip

New features

Walkthrough comment now includes:

  • Possibly related PRs: A list of potentially related PRs to help you recall past context.
  • Suggested labels: CodeRabbit can now suggest labels by learning from your past PRs. You can also provide custom labeling instructions in the UI or configuration file.

Notes:

  • Please share any feedback in the discussion post on our Discord.
  • Possibly related PRs, automatic label suggestions based on past PRs, learnings, and possibly related issues require data opt-in (enabled by default).

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 using 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

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: 0

Outside diff range, codebase verification and nitpick comments (3)
packages/design/smb/src/alert/index.ts (1)

1-5: Potential Impact of Removing the info Icon

The info icon is still referenced in multiple files across the codebase, indicating its continued usage. Removing it from the alert component's icons object could lead to issues in other parts of the application that rely on this icon. Please verify all usages of the info icon to ensure that its removal does not introduce any breaking changes.

  • Files with info icon references:
    • packages/vue/src/alert/src/index.ts
    • packages/vue/src/top-box/index.ts
    • packages/vue/src/tooltip/src/token.ts
    • packages/vue/src/popconfirm/src/index.ts
    • packages/vue/src/notify/index.ts
    • packages/vue/src/modal/src/index.ts
    • packages/vue/src/button/src/index.ts
    • packages/vue-icon/index.ts
    • examples/solid-docs/src/views/button.tsx
    • examples/solid-docs/src/views/alert.tsx
    • examples/sites/demos/pc/app/button/basic-usage.spec.ts
    • examples/sites/demos/pc/app/badge/type.spec.ts
Analysis chain

Verify the impact of removing the info icon.

The removal of the info icon from the alert component's icons object could potentially affect UI elements that rely on this icon. Ensure that this change is intentional and check for any breaking changes in the UI where the info icon was previously used.

Run the following script to verify the usage of the info icon:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `info` icon in the project.

# Test: Search for the `info` icon usage. Expect: No occurrences of the `info` icon.
rg --type typescript -A 5 $'info'

Length of output: 67


Script:

#!/bin/bash
# Description: Verify the usage of the `info` icon in the project.

# Test: Search for the `info` icon usage in TypeScript files. Expect: No occurrences of the `info` icon.
rg --type ts -A 5 $'info'

Length of output: 65736

examples/sites/demos/pc/app/tag/slot-default-composition-api.vue (1)

47-49: Inconsistent border-radius values for .tiny-only-iocn class

The .tiny-only-iocn class is used in two different Vue files with inconsistent border-radius values:

  • examples/sites/demos/pc/app/tag/slot-default-composition-api.vue: border-radius: 0px 8px 0px 8px;
  • examples/sites/demos/pc/app/tag/slot-default.vue: border-radius: 0px 6px 0px 6px;

Please ensure that the border-radius values are consistent across these files unless there is a specific design requirement for the difference.

Analysis chain

Approve the new CSS class styling.

The new CSS class .tiny-only-iocn with specific border-radius styling is correctly implemented to modify the appearance of the tag. Ensure that this styling is consistent across different components and does not conflict with existing styles.

Run the following script to verify the consistency of the new CSS class:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of the new CSS class `.tiny-only-iocn`.

# Test: Search for the CSS class usage. Expect: Consistent styling across different files.
rg --type css -A 5 $'.tiny-only-iocn'

Length of output: 36


Script:

#!/bin/bash
# Description: Verify the usage of the new CSS class `.tiny-only-iocn` across all files.

# Test: Search for the CSS class usage in all files. Expect: Consistent styling across different components.
rg -A 5 $'.tiny-only-iocn'

Length of output: 1547

examples/sites/demos/pc/app/tag/slot-default.vue (1)

55-57: Inconsistent border-radius values for .tiny-only-iocn class.

The CSS class .tiny-only-iocn is used in both slot-default.vue and slot-default-composition-api.vue, but the border-radius values differ between the two files. Ensure that the border-radius values are consistent across these files unless there is a specific design requirement for the difference.

  • examples/sites/demos/pc/app/tag/slot-default.vue: border-radius: 0px 6px 0px 6px;
  • examples/sites/demos/pc/app/tag/slot-default-composition-api.vue: border-radius: 0px 8px 0px 8px;
Analysis chain

Approve the new CSS class styling.

The new CSS class .tiny-only-iocn with specific border-radius styling is correctly implemented to modify the appearance of the tag. Ensure that this styling is consistent across different components and does not conflict with existing styles.

Run the following script to verify the consistency of the new CSS class:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of the new CSS class `.tiny-only-iocn`.

# Test: Search for the CSS class usage. Expect: Consistent styling across different files.
rg --type css -A 5 $'.tiny-only-iocn'

Length of output: 36


Script:

#!/bin/bash
# Description: Verify the consistency of the new CSS class `.tiny-only-iocn` across all file types.

# Test: Search for the CSS class usage in all files. Expect: Consistent styling across different files.
rg -A 5 $'.tiny-only-iocn'

Length of output: 1547

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 363e730 and 0c0606d.

Files selected for processing (4)
  • examples/sites/demos/pc/app/tag/slot-default-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/tag/slot-default.vue (2 hunks)
  • packages/design/smb/src/alert/index.ts (1 hunks)
  • packages/theme/src/tag/smb-theme.js (1 hunks)
Additional comments not posted (3)
examples/sites/demos/pc/app/tag/slot-default-composition-api.vue (1)

11-13: Approve the addition of the new <tiny-tag> component.

The new <tiny-tag> component with a type of "success" and the class tiny-only-iocn is a welcome addition, enhancing the visual representation and functionality. Ensure that this new component is correctly implemented and styled across different platforms and browsers.

Run the following script to verify the correct implementation:

examples/sites/demos/pc/app/tag/slot-default.vue (1)

9-11: Approve the addition of the new <tiny-tag> component.

The new <tiny-tag> component with a size of "small", a type of "success", and the class tiny-only-iocn is a welcome addition, enhancing the visual representation and functionality. Ensure that this new component is correctly implemented and styled across different platforms and browsers.

Run the following script to verify the correct implementation:

Verification successful

Verification successful for the <tiny-tag> component implementation.

The <tiny-tag> component with size="small", type="success", and the class tiny-only-iocn is correctly implemented in examples/sites/demos/pc/app/tag/slot-default.vue. No issues were found with its implementation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correct implementation of the new `<tiny-tag>` component.

# Test: Search for the new `<tiny-tag>` usage. Expect: Correct implementation across different files.
rg --type vue -A 5 $'tiny-tag size="small" type="success"'

Length of output: 87


Script:

#!/bin/bash
# Description: Verify the correct implementation of the new `<tiny-tag>` component.

# Find all .vue files and search for the new `<tiny-tag>` usage.
fd --extension vue --exec rg -Hn -A 5 '<tiny-tag size="small" type="success"' {}

Length of output: 537

packages/theme/src/tag/smb-theme.js (1)

10-10: Verify the new CSS variable for consistency.

The change to the primary background color of tags from var(--ti-common-color-bg-light-normal) to var(--ti-common-color-bg-6) is noted. While this update is likely intended to align with new design guidelines, it's crucial to ensure that the new variable is defined and used consistently across the project.

Verification successful

CSS Variable Usage Verified

The CSS variable --ti-common-color-bg-6 is used consistently across multiple theme files, including tabs, tag, and filter-box components. Its presence with a fallback value in vars.less confirms its role in the design system. The change in packages/theme/src/tag/smb-theme.js aligns with this usage pattern and does not introduce any issues.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition and usage of the new CSS variable.

# Test: Search for the new CSS variable definition. Expect: At least one occurrence.
rg --type css 'var(--ti-common-color-bg-6)'

# Test: Search for the usage of the new CSS variable. Expect: Multiple occurrences.
rg --type css 'var(--ti-common-color-bg-6)'

Length of output: 91


Script:

#!/bin/bash
# Description: Search for the definition of the CSS variable across all file types.

# Test: Search for the definition of the CSS variable. Expect: At least one occurrence.
rg '--ti-common-color-bg-6'

Length of output: 452


Script:

#!/bin/bash
# Description: Correct the search for the definition of the CSS variable across all file types.

# Test: Search for the definition of the CSS variable. Expect: At least one occurrence.
rg '--ti-common-color-bg-6'

Length of output: 452


Script:

#!/bin/bash
# Description: Correct the search for the definition of the CSS variable across all file types.

# Test: Search for the definition of the CSS variable. Expect: At least one occurrence.
rg '--ti-common-color-bg-6'

Length of output: 452


Script:

#!/bin/bash
# Description: Correct the search for the definition of the CSS variable across all file types using the -e option.

# Test: Search for the definition of the CSS variable. Expect: At least one occurrence.
rg -e '--ti-common-color-bg-6'

Length of output: 474

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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0c0606d and efb1cab.

Files selected for processing (5)
  • examples/sites/demos/pc/app/tag/slot-default-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/tag/slot-default.vue (2 hunks)
  • packages/theme/src/tag/index.less (1 hunks)
  • packages/theme/src/tag/smb-theme.js (1 hunks)
  • packages/theme/src/tag/vars.less (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/theme/src/tag/vars.less
Files skipped from review as they are similar to previous changes (3)
  • examples/sites/demos/pc/app/tag/slot-default-composition-api.vue
  • examples/sites/demos/pc/app/tag/slot-default.vue
  • packages/theme/src/tag/smb-theme.js
Additional comments not posted (1)
packages/theme/src/tag/index.less (1)

261-261: Good use of CSS variables for theming.

The change to use a CSS variable for the disabled tag background color enhances maintainability and flexibility. Ensure that the variable --ti-tag-disabled-background-color is defined elsewhere in the stylesheets or theme configuration to avoid any runtime issues.

Run the following script to verify the definition of the CSS variable:

Verification successful

CSS Variable Definition Verified

The CSS variable --ti-tag-disabled-background-color is correctly defined in packages/theme/src/tag/vars.less and used in packages/theme/src/tag/index.less. This ensures maintainability and theming flexibility without runtime issues.

  • Definition found in: packages/theme/src/tag/vars.less
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition of the CSS variable `--ti-tag-disabled-background-color`.

# Test: Search for the variable definition. Expect: At least one occurrence.
rg --type css --type less --type scss $'--ti-tag-disabled-background-color'

Length of output: 511


Script:

#!/bin/bash
# Description: Verify the definition of the CSS variable `--ti-tag-disabled-background-color`.

# Test: Search for the variable definition. Expect: At least one occurrence.
rg --type css --type less --type scss '--ti-tag-disabled-background-color'

Length of output: 511


Script:

#!/bin/bash
# Description: Verify the definition of the CSS variable `--ti-tag-disabled-background-color`.

# Test: Search for the variable definition. Expect: At least one occurrence.
rg --type css --type less --type scss '\-\-ti-tag-disabled-background-color'

Length of output: 107


Script:

#!/bin/bash
# Description: Verify the definition of the CSS variable `--ti-tag-disabled-background-color`.

# Test: Search for the variable definition. Expect: At least one occurrence.
rg --type css --type less '\-\-ti-tag-disabled-background-color'

Length of output: 255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants