feat(carousel): [carousel] modify smb theme

[James-9696]

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

  • Enhanced visual appearance of carousel items with updated background colors for better contrast.
  • Improved styling of carousel item titles, including adjustments to text color, opacity, and padding.
  • Refined layout of carousel indicators for better alignment and spacing.

• Bug Fixes

  • Addressed spacing issues in carousel indicators to ensure consistent visual alignment.

These updates collectively enhance the aesthetic and usability of the carousel component.

[coderabbitai]

Walkthrough

The changes in this pull request primarily involve styling adjustments to the carousel component across multiple Vue.js and LESS files. Modifications include alterations to background colors, title text properties, and spacing for carousel items and indicators. These updates aim to enhance the visual presentation without affecting the underlying functionality of the carousel.

Changes

Files	Change Summary
examples/sites/demos/pc/app/carousel/*.vue	Updated background color for every second carousel item from #fafafa to #f0f0f0.
packages/theme/src/carousel-item/index.less	Adjusted title text color, opacity to 0.3, and increased padding from 12px to 16px.
packages/theme/src/carousel-item/vars.less	Modified title height to medium, simplified text color, and changed background color to primary.
packages/theme/src/carousel/index.less	Added margin adjustment for the last carousel indicator and increased bottom margin from 8px to 16px.
packages/theme/src/carousel/vars.less	Changed non-active indicator background color to #e6e6e6 and adjusted active indicator sizes.

Possibly related PRs

• feat(carousel): [carousel]modify smb theme and add props #2125: Modifications to the carousel component, including styling adjustments and the addition of new properties, directly related to the carousel's visual presentation.
• feat(modal): [modal] modify smb theme #2146: Focuses on modifying the SMB theme for the modal component, indicating ongoing theme adaptations across components, including visual aspects that could align with the carousel's styling updates.

Suggested labels

enhancement

Suggested reviewers

• zzcr

🐇 In the carousel's dance, colors bright and bold,
A hop of change, a story told.
With titles that shimmer, and spacing just right,
Our visuals now sparkle, a delightful sight!
So let’s twirl and whirl, in style we prance,
In this vibrant world, let’s take a chance! 🌈

---

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 577d509 and 9bfddd0.

Files selected for processing (6)

• examples/sites/demos/pc/app/carousel/show-title-composition-api.vue (1 hunks)
• examples/sites/demos/pc/app/carousel/show-title.vue (1 hunks)
• packages/theme/src/carousel-item/index.less (1 hunks)
• packages/theme/src/carousel-item/vars.less (1 hunks)
• packages/theme/src/carousel/index.less (2 hunks)
• packages/theme/src/carousel/vars.less (2 hunks)

Files skipped from review due to trivial changes (2)

• examples/sites/demos/pc/app/carousel/show-title-composition-api.vue
• examples/sites/demos/pc/app/carousel/show-title.vue

Additional comments not posted (12)

packages/theme/src/carousel-item/vars.less (3)

15-15: Verify the value of --ti-common-color-light.

The change simplifies the text color of the carousel item title by removing the fallback value. However, ensure that --ti-common-color-light is defined with an appropriate value to maintain consistency.

Run the following script to verify the value of --ti-common-color-light:

#!/bin/bash
# Description: Verify the value of `--ti-common-color-light` is defined.

# Test: Search for the definition of `--ti-common-color-light`. Expect: At least one occurrence.
rg --type less $'--ti-common-color-light:'

---

14-14: Verify the value of --ti-common-size-height-medium.

The change improves the visual prominence of the carousel item title by increasing its height from a small size to a medium size. However, ensure that --ti-common-size-height-medium is defined with an appropriate value to maintain consistency.

Run the following script to verify the value of --ti-common-size-height-medium:

Verification successful

---

Verification successful: --ti-common-size-height-medium is properly defined.

The variable --ti-common-size-height-medium is correctly defined in the theme with a value of 40px. This definition is found in packages/theme/src/base/basic-var.less. The change in the carousel-item component to use this variable is consistent with the theme's design system.

Additional observations:

• The variable is widely used across various components, ensuring design consistency.
• There's an older definition in old-basic-vars.less which might be for backwards compatibility.
• Related size variables (large, small, normal, mini, minor) are also defined, providing a complete set of size options.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the value of `--ti-common-size-height-medium` is defined.

# Test: Search for the definition of `--ti-common-size-height-medium`. Expect: At least one occurrence.
rg --type less $'--ti-common-size-height-medium:'

Length of output: 974

---

Script:

#!/bin/bash
# Description: Verify the value of `--ti-common-size-height-medium` is defined.

# Test: Search for the definition of `--ti-common-size-height-medium`. Expect: At least one occurrence.
rg --type less '--ti-common-size-height-medium:'

# Fallback: Search for any `--ti-common-size-height` definitions
echo "Fallback search for --ti-common-size-height definitions:"
rg --type less '--ti-common-size-height-'

Length of output: 1074

---

Script:

#!/bin/bash
# Description: Verify the value of `--ti-common-size-height-medium` is defined.

# Test: Search for the definition of `--ti-common-size-height-medium`. Expect: At least one occurrence.
rg --type less -e '--ti-common-size-height-medium:'

# Fallback: Search for any `--ti-common-size-height` definitions
echo "Fallback search for --ti-common-size-height definitions:"
rg --type less -e '--ti-common-size-height-'

Length of output: 10508

---

16-16: Verify the value of --ti-common-color-bg-primary and its impact on the color scheme.

The change enhances the visual appeal of the carousel item title by using a primary color for the background instead of a disabled state. However, ensure that --ti-common-color-bg-primary is defined with an appropriate value and that the change aligns with the overall color scheme and contrast of the carousel component.

Run the following script to verify the value of --ti-common-color-bg-primary and its usage in the codebase:

Verification successful

---

Approve the change: --ti-common-color-bg-primary is properly defined and consistently used.

The change to use --ti-common-color-bg-primary for the carousel item title background is appropriate. This variable is defined in packages/theme/src/base/basic-var.less as:

--ti-common-color-bg-primary: var(--ti-base-color-bg-3); // smb 新增

It's used consistently across multiple components, including milestones and the base variables. The layered approach (using --ti-base-color-bg-3) allows for easier theme customization. The change aligns with the variable's intended use as a primary background color.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the value of `--ti-common-color-bg-primary` is defined and analyze its usage.

# Test 1: Search for the definition of `--ti-common-color-bg-primary`. Expect: At least one occurrence.
rg --type less $'--ti-common-color-bg-primary:'

# Test 2: Search for the usage of `--ti-common-color-bg-primary`. Expect: Occurrences in relevant components.
rg --type less $'var\(--ti-common-color-bg-primary\)'

Length of output: 974

---

Script:

#!/bin/bash
# Description: Verify the definition of `--ti-common-color-bg-primary`

# Search for the definition of --ti-common-color-bg-primary
rg --type less '--ti-common-color-bg-primary:'

# Search for any related color definitions that might affect --ti-common-color-bg-primary
rg --type less '--ti-common-color-bg-.*:'

Length of output: 949

---

Script:

#!/bin/bash
# Description: Verify the definition of `--ti-common-color-bg-primary` and related color variables

# Search for the definition of --ti-common-color-bg-primary
rg --type less '^\s*--ti-common-color-bg-primary:'

# Search for related color definitions
rg --type less '^\s*--ti-common-color-bg-.*:'

# Search for imports or includes that might contain variable definitions
rg --type less '@import.*color'
rg --type less '@include.*color'

# Search for theme-related files that might contain color definitions
fd -e less | grep -i 'color\|theme'

Length of output: 35502

packages/theme/src/carousel-item/index.less (3)

41-41: Verify the impact of removing the default fallback color.

Removing the default fallback color value for the title text color provides flexibility for customization through the --ti-carousel-item-title-text-color custom property. However, it's important to ensure that the custom property is consistently defined to avoid falling back to the browser's default text color.

Consider adding a fallback color value to ensure a consistent appearance across different environments. For example:

-color: var(--ti-carousel-item-title-text-color);
+color: var(--ti-carousel-item-title-text-color, #fff);

---

44-44: Verify the impact of the opacity change on title text visibility.

Setting the opacity of the .item-title element to 0.3 will make the title text more transparent. While this can create a subtle visual effect, it's important to ensure that the title text remains easily readable and visible against the background.

Please review the opacity value in the context of the carousel item's background color and the title text color to ensure sufficient contrast and readability. Consider adjusting the opacity value if necessary to maintain optimal visibility.

---

47-47: Verify the impact of the padding change on the title layout.

Increasing the padding for the span element within the .item-title element from 12px to 16px will add more space around the title text. While this can improve the visual spacing, it's important to consider the impact on the overall layout and appearance of the title.

Please review the padding value in the context of the available space for the title and the expected title text length. Ensure that the increased padding does not cause the title text to wrap or overflow unintentionally. Consider adjusting the padding value or implementing responsive styles if necessary to maintain a clean and readable title layout across different screen sizes.

packages/theme/src/carousel/vars.less (4)

47-47: Verify the design intention and impact of changing the non-active indicator button color to a fixed value.

The change from a variable reference to a fixed color value for --ti-carousel-indicator-button-bg-color reduces flexibility in theming the non-active indicator buttons. However, this may be an intentional design decision to standardize the color across themes.

Please confirm if this change aligns with the intended design and consider its impact on the ability to customize the non-active indicator button color in different themes.

---

51-51: Verify the design intention and impact of reducing the active indicator width.

The change reduces the width of the active indicator from --ti-common-size-5x to --ti-common-size-3x, which may impact its visual prominence. However, the change still uses a variable reference, maintaining some flexibility in sizing.

Please confirm if this change aligns with the intended design and consider its impact on the visual prominence of the active indicator, especially in relation to the non-active indicators.

---

61-61: LGTM!

The change to the active button width maintains consistency with the previous change to the active indicator width. This consistency is important for a cohesive design.

---

69-69: Verify the design intention and impact of changing the active outside button background color.

The change replaces the specific --ti-carousel-outside-button-active-bg-color variable with the more generic --ti-common-color-text-secondary variable for the active outside button background color. This may alter the appearance of the active outside button in relation to the overall theme and reduces flexibility in specifically theming this color.

Please confirm if this change aligns with the intended design and consider its impact on the ability to customize the active outside button background color to differentiate it from other secondary text elements.

packages/theme/src/carousel/index.less (2)

131-133: LGTM!

The change to remove the right margin from the last carousel indicator improves the visual alignment and spacing. The implementation looks good.

---

219-219: Looks good!

Increasing the bottom margin of the carousel indicators from 8px to 16px enhances the layout and visual separation of the carousel. The change is straightforward and effective.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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>, please review it.
  -- 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  -- @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.