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

refactor(wizard): [wizard] refactor the Wizard Theme #2279

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

chenxi-20
Copy link
Collaborator

@chenxi-20 chenxi-20 commented Oct 16, 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 new mixins for styling wizard elements, enhancing visual customization options.
  • Bug Fixes
    • Corrected a typo in class names to ensure proper styling and functionality across components.
  • Style
    • Updated CSS variable naming conventions to align with a new design system, improving consistency in styling.
    • Adjusted CSS properties for various elements within the wizard for better visual representation.
  • Tests
    • Modified test cases to reflect the corrected class names and updated CSS properties, ensuring accurate component testing.

@chenxi-20 chenxi-20 added the refactoring Refactoring (重构) label Oct 16, 2024
Copy link

coderabbitai bot commented Oct 16, 2024

Walkthrough

The pull request introduces several changes to the wizard component's styling and structure. Key modifications include correcting a typo in class names, updating CSS variable prefixes from ti- to tv-, and enhancing styling mixins. The wizard.less file was removed, consolidating mixins into the index.less file. Additionally, test cases were updated to reflect the corrected class names.

Changes

File Path Change Summary
packages/theme-saas/src/wizard/index.less Corrected class name __nomarl to __normal, added new mixins for icon and line colors, updated calls to .inject-Wizard-vars(), enhanced wizard state styles.
packages/theme/src/mixins/wizard.less Removed file containing mixins for wizard components.
packages/theme/src/wizard/old-theme.js Renamed CSS variable keys in tinyWizardOldTheme from ti- to tv-.
packages/theme/src/wizard/vars.less Renamed various CSS variables from --ti- to --tv-, updated styles, and removed some variables.
packages/vue/src/wizard/__tests__/wizard.test.tsx Corrected class name in test cases from .tiny-wizard__nomarl to .tiny-wizard__normal.
packages/vue/src/wizard/src/pc.vue Corrected class name in template from tiny-wizard__nomarl to tiny-wizard__normal.
examples/sites/demos/pc/app/wizard/base-usage.spec.ts Adjusted expected CSS properties for chart items, including width and height changes.
examples/sites/demos/pc/app/wizard/time-line-flow.spec.ts Updated expected CSS properties for timeline wizard elements, including width and background color changes.
examples/sites/demos/pc/app/wizard/vertical.spec.ts Modified expected CSS properties for vertical wizard elements, including node lines and icons.

Possibly related PRs

Suggested reviewers

  • zzcr

🐇 "In the land of wizards, where styles do play,
A typo was caught, now normal's here to stay.
With colors and lines, all fresh and bright,
Our wizard's now ready, a true delight!
So hop along, friends, let’s cheer with glee,
For the magic of changes, as fine as can be!" 🐇✨


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

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 and nitpick comments (9)
packages/vue/src/wizard/__tests__/wizard.test.tsx (1)

Line range hint 1-124: Consider refactoring repeated class names for improved maintainability.

While the current changes correctly fix the typo, consider refactoring repeated class names into constants. This approach would make future updates easier and reduce the risk of inconsistencies. For example:

const WIZARD_NORMAL_CLASS = '.tiny-wizard__normal';

// Then use it in your tests like this:
expect(wrapper.find(WIZARD_NORMAL_CLASS).exists()).toBeTruthy()

This refactoring would apply to other repeated class names as well, improving overall test maintainability.

packages/theme/src/wizard/vars.less (2)

35-35: Enhanced spacing consistency and flexibility.

The use of standardized --tv-space-* variables for spacing and the calc() function for size calculations improves consistency and flexibility in the theme.

Consider using calc() for all size calculations to make the theme even more flexible. For example, line 35 could be changed to:

--- --tv-Wizard-lines-width: 30px;
+++ --tv-Wizard-lines-width: calc(var(--tv-space-base, 4px) * 7.5);

This would allow for easier scaling of the entire theme by adjusting the base spacing variable.

Also applies to: 37-37, 61-61, 63-63, 65-65, 77-77, 79-79, 81-81, 107-107, 113-113, 117-117, 127-127, 129-129, 131-131


35-35: Consider further improvements for consistency and code cleanliness.

  1. Some variables still have hardcoded values (e.g., lines 35, 69, 95, 113). Consider replacing these with calculated values or standardized variables for better theme flexibility.

  2. There are commented out properties marked with "(hide)" (e.g., lines 71-75, 125). These should be reviewed and either removed if they are no longer needed or uncommented if they are still required.

Please review these items and make appropriate adjustments to further improve the consistency and cleanliness of the code.

Also applies to: 69-69, 71-75, 95-95, 113-113, 125-125

packages/vue/src/wizard/src/pc.vue (2)

Line range hint 132-132: Consider adding type definitions for props.

To improve type safety and developer experience, consider adding explicit type definitions for the props, especially for the 'data' prop. This will help catch potential type-related issues early in the development process.

Here's an example of how you could define the prop types:

import { PropType } from 'vue'

// Define interfaces for your data structures
interface WizardStep {
  name: string
  status: string
  // Add other properties as needed
}

// In your component definition
props: {
  data: {
    type: Array as PropType<WizardStep[]>,
    required: true
  },
  pageGuide: Boolean,
  timeLineFlow: Boolean,
  vertical: Boolean
  // ... other props
}

Line range hint 16-121: Consider using i18n for all text in the template.

The template contains a mix of English and non-English text. To improve maintainability and support internationalization, consider using a translation system (i18n) for all text in the template.

Here's an example of how you could refactor some of the text using i18n:

<template>
  <!-- ... -->
  <tiny-button :reset-time="0" @click="lastStepHandle">{{ $t('wizard.previousStep') }}</tiny-button>
  <tiny-button :reset-time="0" @click="btnSaveHandle">{{ $t('wizard.save') }}</tiny-button>
  <tiny-button :reset-time="0" @click="nextStepHandle">{{ $t('wizard.nextStep') }}</tiny-button>
  <tiny-button :reset-time="0" v-if="state.submitShow" @click="submitHandle">{{ $t('wizard.submit') }}</tiny-button>
  <!-- ... -->
</template>

Ensure that you have the corresponding translations set up in your i18n configuration.

packages/theme-saas/src/wizard/index.less (1)

Line range hint 1-624: Summary and suggestion for improvement

The changes in this file consistently correct a typo in class names from __nomarl to __normal. These corrections are necessary and improve the accuracy of the CSS selectors.

While reviewing the file, I noticed that it's quite large and contains complex nested selectors. To improve maintainability, consider splitting this file into smaller, more focused files (e.g., separating normal and vertical styles).

Would you like assistance in refactoring this file into smaller, more manageable components?

packages/theme/src/wizard/index.less (3)

22-22: Translate comment to English for consistency

The comment on line 22, /** 节点圆圈场景 */, is in Chinese. To maintain consistency and improve readability for all contributors, please consider translating it to English.


29-29: Translate comment to English for consistency

The comment on line 29, /** 线条场景 */, is in Chinese. Please consider translating it to English to maintain uniformity.


302-317: Set cursor to pointer for interactive elements

The cursor style for icons, names, dates, and SVG elements is now set to pointer to indicate interactivity. Ensure that this aligns with user expectations and that interactive elements respond appropriately.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 766f054 and 7672953.

📒 Files selected for processing (7)
  • packages/theme-saas/src/wizard/index.less (3 hunks)
  • packages/theme/src/mixins/wizard.less (0 hunks)
  • packages/theme/src/wizard/index.less (5 hunks)
  • packages/theme/src/wizard/old-theme.js (1 hunks)
  • packages/theme/src/wizard/vars.less (1 hunks)
  • packages/vue/src/wizard/tests/wizard.test.tsx (1 hunks)
  • packages/vue/src/wizard/src/pc.vue (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/theme/src/mixins/wizard.less
🧰 Additional context used
🔇 Additional comments (26)
packages/theme/src/wizard/old-theme.js (2)

2-50: Verify the scope of 'ti-' to 'tv-' renaming across the project.

While the changes in this file are consistent, it's important to ensure that this renaming is applied uniformly across the entire project. This change may have wider implications for components or styles that depend on these theme variables.

Please run the following script to check for any inconsistencies in the prefix usage across the project:

#!/bin/bash
# Description: Check for inconsistencies in prefix usage across the project

# Test 1: Count occurrences of 'ti-' and 'tv-' prefixes in theme-related files
echo "Occurrences of 'ti-' prefix in theme files:"
rg --type js --type less 'ti-' packages/theme/src | wc -l

echo "Occurrences of 'tv-' prefix in theme files:"
rg --type js --type less 'tv-' packages/theme/src | wc -l

# Test 2: Check for mixed usage of 'ti-' and 'tv-' prefixes in the same file
echo "Files with mixed 'ti-' and 'tv-' prefix usage:"
rg --type js --type less --files-with-matches 'ti-' packages/theme/src | xargs rg --type js --type less 'tv-' | cut -d':' -f1 | sort | uniq

If these tests reveal any inconsistencies or mixed usage, please address them to maintain uniformity across the project.


2-50: Consistent renaming of theme variables from 'ti-' to 'tv-' prefix.

The change consistently renames all theme variables from 'ti-' prefix to 'tv-' prefix while preserving their values. This systematic update suggests a deliberate change in naming convention, likely part of a larger refactoring effort.

To ensure this change doesn't introduce any regressions, please run the following script to check for any remaining 'ti-' prefixed variables in the wizard-related files:

Additionally, to verify the impact on the rest of the codebase:

If these scripts return any results, please update those occurrences to use the new 'tv-' prefix.

packages/vue/src/wizard/__tests__/wizard.test.tsx (2)

97-97: Excellent correction of the class name typo.

The change from .tiny-wizard__nomarl to .tiny-wizard__normal fixes a critical typo in the class name assertion. This correction ensures that the test accurately checks for the presence of the correct class in the rendered Wizard component.


102-102: Consistent correction of the class name typo.

This change mirrors the previous correction, changing .tiny-wizard__nomarl to .tiny-wizard__normal. It maintains consistency across test cases and ensures accurate testing of the page-guide flow.

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

15-15: Improved consistency in font sizes and colors.

The updates to font size and color variables now reference standardized --tv-font-size-* and --tv-color-* variables. This change enhances theme consistency and simplifies future maintenance.

Also applies to: 17-17, 19-19, 21-21, 27-27, 29-29, 31-31, 33-33, 85-85, 87-87, 89-89, 91-91, 97-97, 99-99, 101-101, 119-119


23-23: Standardized border properties for improved consistency.

The updates to border radius and width variables now reference standardized --tv-border-radius-* and --tv-border-width variables. This change enhances theme consistency for border styles and simplifies future maintenance.

Also applies to: 59-59, 67-67


13-131: Overall improvement in theme consistency and maintainability.

The changes made to this file significantly improve the consistency and maintainability of the Wizard component's styling. The standardization of variable names and the use of common theme variables for colors, fonts, and spacing will make it easier to manage and update the theme in the future.

While there are a few minor areas that could be further improved (as noted in previous comments), the overall direction of these changes is positive and aligns well with best practices for CSS theming.

Great job on this refactoring effort!


13-131: Consistent renaming of CSS variables approved.

The renaming of all CSS variables from --ti- prefix to --tv- prefix is consistent and improves the overall naming convention of the project.

To ensure that these changes don't break existing styles, please run the following script to check for any remaining --ti- prefixed variables in other files:

packages/vue/src/wizard/src/pc.vue (1)

15-15: Approve typo correction in class name.

The change from tiny-wizard__nomarl to tiny-wizard__normal corrects a typo in the class name. This improvement enhances code readability and consistency.

To ensure this change doesn't break any existing styles, please run the following script to check for any CSS rules that might have been using the old class name:

If the old class name is found, update those CSS rules to use the new class name tiny-wizard__normal.

packages/theme-saas/src/wizard/index.less (2)

11-11: Approve typo correction and suggest verification

The typo in the class name has been correctly fixed from __nomarl to __normal. This change is necessary for proper CSS targeting.

To ensure this change doesn't break existing markup, please run the following command to check for any remaining instances of the misspelled class:


287-287: Approve consistent typo correction

The typo in the class name has been consistently corrected from __nomarl to __normal in this nested selector as well. This change ensures proper styling for the normal state of the wizard component.

To ensure all instances of this typo have been addressed, please run the following command to search for any remaining occurrences in the entire codebase:

✅ Verification successful

Typo Correction Verified

No remaining instances of "nomarl" were found in the codebase. The typo has been successfully corrected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining instances of the misspelled word in all file types
rg -i "nomarl" --type-not binary

Length of output: 151


Script:

#!/bin/bash
# Search for any remaining instances of the misspelled word in all text-based file types, excluding binary files
rg -i "nomarl" --binary-files=without-match

Length of output: 484


Script:

#!/bin/bash
# Search for any remaining instances of the misspelled word in all text-based file types, excluding binary files
rg -i "nomarl"

Length of output: 14

packages/theme/src/wizard/index.less (15)

23-27: New mixin .wizard-icon-color added

The mixin .wizard-icon-color(@color, @bgColor, @borderColor) is well-defined and enhances code reusability for icon styling.


30-33: New mixin .wizard-line-color added

The mixin .wizard-line-color(@color) is appropriately defined for line styling and improves maintainability.


36-36: Updated variable injection method

Replaced .component-css-vars-wizard(); with .inject-Wizard-vars();. Ensure that the new mixin is correctly defined and imported.


64-65: Adjusted margin for node titles

The margin-top for .@{wizard-prefix-cls}__name has been updated to var(--tv-Wizard-node-title-margin-top). Verify that this change maintains the desired spacing in the UI.


76-77: Adjusted margin for button group

The margin-top for .@{wizard-prefix-cls}__button is now set to var(--tv-Wizard-button-group-margin-top). Ensure this aligns correctly with the design specifications.


80-80: Updated button spacing

The margin between buttons has been updated using var(--tv-Wizard-button-margin-left). Verify that the spacing appears consistent across different browsers.


85-206: Enhanced vertical mode styling

Significant updates have been made to the vertical mode styles, including new variables and layouts for different states like is-doing and is-ready. Please ensure these changes are thoroughly tested for visual consistency and responsiveness.


207-245: Added time-line flow styles

New styles for the time-line flow scenario in vertical mode have been introduced. Verify that the time-line displays correctly and functions as intended.


246-276: Adjusted date styling for vertical time-line

The date component styles have been updated for better alignment and readability. Ensure that dates are displayed correctly and do not overlap with other elements.


Line range hint 277-301: Updated global node styles

Changes have been made to the global node styles, including states for is-ready and adjustments to text colors. Verify that these updates reflect accurately in the component's appearance.


318-344: Consistent styling for steps and child components

Styles for .@{wizard-prefix-cls}__steps-item and .@{wizard-prefix-cls}__chart-children have been standardized across different states. Verify that these components render correctly and maintain visual consistency.


Line range hint 345-389: Improved detail panel styling on hover

The detail panel that appears on hover now has enhanced styling, including background color and pseudo-elements. Ensure that the hover effects display correctly and enhance the user experience.


390-410: Updated default node circle styles

The default styles for node circles, including sizes, colors, and borders, have been updated. Verify that these nodes appear correctly in all components and do not affect layout.


423-437: Refined line styles between nodes

Line styles connecting the nodes have been refined for better visual appeal. Ensure that lines render correctly and connect nodes seamlessly.


Line range hint 438-442: Adjusted user contact information display

Updates have been made to hide unnecessary elements like user heads and SVGs within the user contact component. Confirm that only relevant information is displayed to the user.

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

🧹 Outside diff range and nitpick comments (4)
examples/sites/demos/pc/app/wizard/vertical.spec.ts (2)

26-28: Unchanged colors for the third node

The color values for the third node (index 3) appear unchanged:

  1. The line background color remains rgb(219, 219, 219).
  2. The icon background color remains rgb(255, 255, 255).

While these lines have been modified in the diff, the actual color values haven't changed. This might be an oversight or an intentional reaffirmation of the existing colors. Consider the following:

  1. If changes were intended, ensure the correct new colors are applied.
  2. If no changes were intended, these lines could be reverted to reduce noise in the diff.

Line range hint 1-34: Summary of changes to the vertical wizard test

This update to the vertical wizard test reflects significant changes to the component's visual design. Key points to consider:

  1. The overall structure of the wizard has been made more compact, with thinner lines and reduced vertical spacing.
  2. Icon sizes have increased, potentially improving visibility or touch targets.
  3. The color scheme has shifted dramatically, moving away from blue tones to a more monochromatic palette with dark gray lines and white icons.
  4. The last node in the wizard now has a transparent line, which may affect the visual indication of completion.

These changes suggest a substantial redesign of the wizard component. To ensure the quality and consistency of these changes:

  1. Verify that these visual updates are reflected in the component's implementation, not just in the test expectations.
  2. Consider updating the component's documentation to reflect the new visual design.
  3. Ensure that these changes maintain or improve the component's accessibility and usability across different themes and color modes.
  4. If these changes are part of a larger design system update, confirm that they align with other components and overall design guidelines.

Consider adding visual regression tests to catch any unintended changes to the component's appearance in future updates.

examples/sites/demos/pc/app/wizard/time-line-flow.spec.ts (1)

36-47: Summary of visual changes to the wizard component

The changes in this test file reflect significant visual updates to the wizard component:

  1. Thinner lines (from 4px to 1px) for both childNodeLines and nodeLines.
  2. Larger icons (from 16x20px to 32x32px) for the first two nodeIcons.
  3. A visible last childNodeLine (changed from transparent to dark gray/black).

These changes will noticeably alter the appearance of the wizard. Please ensure that:

  1. All changes are intentional and align with the updated design specifications.
  2. The changes are consistently applied across all relevant components.
  3. Any potential layout implications due to size changes have been considered and addressed.
  4. The corresponding CSS files have been updated to reflect these changes.

Consider creating a separate visual regression test suite to catch any unintended visual changes in the future. This could help maintain the consistency of the wizard's appearance across different components and future updates.

packages/theme/src/wizard/vars.less (1)

75-75: Typographical error in comment.

In the comment on line 75:

// 垂直模式文件外边距(hide)

The word "文件" (file) seems incorrect in this context. It should likely be "文本" (text) to match the surrounding comments.

Corrected comment:

// 垂直模式文本外边距(hide)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7672953 and 16fbd10.

📒 Files selected for processing (4)
  • examples/sites/demos/pc/app/wizard/base-usage.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/wizard/time-line-flow.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/wizard/vertical.spec.ts (1 hunks)
  • packages/theme/src/wizard/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (14)
examples/sites/demos/pc/app/wizard/vertical.spec.ts (3)

24-25: Visual changes to the last node of the wizard

The last node of the wizard has undergone significant visual changes:

  1. The node line is now transparent (rgba(0, 0, 0, 0)) instead of gray (rgb(194, 196, 199)).
  2. The icon background is slightly lighter (rgb(240, 240, 240) instead of rgb(223, 225, 230)).

These changes may affect the visibility and user perception of the wizard's completion state. Ensure that these modifications align with the intended design and maintain clear visual cues for the user.

To confirm these color changes are consistently applied, run the following script:

#!/bin/bash
# Description: Verify the new colors are consistently applied across the codebase.

# Test: Search for any remaining instances of the old colors.
rg --type less --type vue 'rgb\(194, 196, 199\)|rgb\(223, 225, 230\)' src/wizard

30-31: Major color scheme changes for wizard nodes

The color scheme for the wizard nodes (excluding the last and third nodes) has undergone significant changes:

  1. Node lines have changed from blue (rgb(94, 124, 224)) to very dark gray (rgb(25, 25, 25)).
  2. Node icons have changed from blue (rgb(94, 124, 224)) to white (rgb(255, 255, 255)).

These changes represent a substantial shift in the visual design of the wizard. Consider the following implications:

  1. Ensure that these changes align with the overall design system and maintain visual consistency with other components.
  2. Verify that the new color scheme provides sufficient contrast for accessibility, especially between the dark gray lines and the background.
  3. Confirm that the white icons on potentially varying backgrounds maintain good visibility across different themes or color modes.

To ensure these color changes are consistently applied and to check for any potential accessibility issues, run the following script:

#!/bin/bash
# Description: Verify color consistency and check contrast ratios.

# Test: Search for any remaining instances of the old blue color.
rg --type less --type vue 'rgb\(94, 124, 224\)' src/wizard

# Note: For a comprehensive accessibility check, consider using a tool like 'pa11y' 
# or 'axe-core' to analyze the contrast ratios in the actual rendered component.

19-21: Significant changes in wizard dimensions

The changes in the wizard's visual elements are noteworthy:

  1. Node lines are now thinner (1px instead of 4px).
  2. Vertical spacing between nodes is reduced (60px instead of 88px).
  3. Node icons are larger (32px instead of 20px).

These modifications suggest a more compact overall design with more prominent icons. Ensure that these changes align with the intended design updates for the wizard component.

To confirm these changes are consistent across the codebase, run the following script:

✅ Verification successful

Wizard dimensions updated consistently

All old dimension values ('4px', '88px', '20px') have been successfully removed from the examples/sites/demos/pc/app/wizard directory. The new CSS properties are consistently applied, ensuring the wizard's design aligns with the latest specifications.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new dimensions are consistently applied across the codebase.

# Test: Search for any remaining instances of the old dimensions.
rg --type less --type vue '4px|88px|20px' src/wizard

Length of output: 88


Script:

#!/bin/bash
# Description: Verify the new dimensions are consistently applied across .less and .vue files in the src/wizard directory.

# Search for any remaining instances of the old dimensions in .less and .vue files.
find src/wizard -type f \( -name "*.less" -o -name "*.vue" \) -exec grep -E '4px|88px|20px' {} +

Length of output: 148


Script:

#!/bin/bash
# Description: Verify the new dimensions are consistently applied across .less and .vue files in the examples/sites/demos/pc/app/wizard directory.

# Search for any remaining instances of the old dimensions in .less and .vue files.
find examples/sites/demos/pc/app/wizard -type f \( -name "*.less" -o -name "*.vue" \) -exec grep -E '4px|88px|20px' {} +

Length of output: 126

examples/sites/demos/pc/app/wizard/base-usage.spec.ts (6)

22-23: LGTM. Verify visual appearance.

The changes to the chart item dimensions (width: 100px, height: 32px) look good. These updates likely reflect a design change for the wizard component.

To ensure these changes align with the intended design, please verify the visual appearance of the wizard component in the browser.


30-30: Verify the intended behavior of wizard steps.

The condition for applying the itemStatus class has been changed from if (i < 2) to if (i < 1). This reduces the number of steps that receive the "ready" or "doing" status.

Please confirm if this change is intentional and aligns with the expected behavior of the wizard component. Consider the following questions:

  1. Is only the first step supposed to have a special status now?
  2. How does this change affect the user's perception of the wizard's progress?
  3. Are there any UI/UX implications that need to be addressed?

32-33: LGTM. Verify visual appearance of the first step.

The background color changes for the first step (when i < 1) look good:

  • First item: transparent (rgba(0, 0, 0, 0))
  • Middle item: white (rgb(255, 255, 255))

Please ensure these color changes create the intended visual effect for the first step of the wizard in the browser.


34-36: LGTM. Verify visual states of the wizard steps.

The new condition and background color changes for the second step (when i === 1) look good:

  • First item: very dark gray (rgb(25, 25, 25))
  • Middle item: white (rgb(255, 255, 255))

Please confirm that these color changes create the intended visual states for the wizard steps:

  1. Does the dark gray background for the first item of the second step correctly represent its state (e.g., completed or active)?
  2. Is the contrast between the dark gray and white backgrounds sufficient for good visibility?
  3. Do these changes align with the overall design system and accessibility guidelines?

38-39: LGTM. Verify overall visual flow of the wizard.

The background color changes for steps after the second step (when i > 1) look good:

  • First item: light gray (rgb(219, 219, 219))
  • Middle item: very light gray (rgb(240, 240, 240))

Please ensure these color changes create the intended visual flow for the wizard:

  1. Do the light gray colors effectively represent inactive or future steps?
  2. Is there a clear visual progression from active to inactive steps?
  3. Does the overall color scheme of the wizard (including all states) provide a cohesive and intuitive user experience?

22-39: Summary: Verify overall wizard component behavior and appearance

The changes to the wizard component test file look good overall. They include updates to dimensions, conditional logic, and background colors for different steps. These modifications likely reflect a design update for the wizard component.

To ensure these changes meet the intended design and functionality:

  1. Run the tests and verify they pass with the updated expectations.
  2. Manually test the wizard component in the browser to confirm:
    • The visual appearance matches the new specifications.
    • The progression of step states (ready, doing, inactive) behaves as expected.
    • The overall user experience is intuitive and accessible.
  3. If possible, cross-reference these changes with any related design documents or specifications to ensure alignment.
examples/sites/demos/pc/app/wizard/time-line-flow.spec.ts (4)

36-36: Verify the design change for childNodeLines width.

The width of childNodeLines has been reduced from 4px to 1px. This change affects the visual representation of the timeline elements, making the lines thinner.

Please confirm that this change is intentional and consistent with the updated design specifications. Also, ensure that this change is reflected in the corresponding CSS files.

#!/bin/bash
# Description: Verify the width change in CSS files

# Test: Search for childNodeLines width in CSS files
rg --type css -i 'childNodeLines.*width'

45-46: Verify the size increase for nodeIcons.

The dimensions of the first two nodeIcons have been doubled from 16x20px to 32x32px. This significant size increase will affect the visual prominence of these icons in the wizard.

Please confirm the following:

  1. Is this size increase intentional and consistent with the updated design?
  2. Are there any layout implications due to this size change?
  3. Should the third nodeIcon (if it exists) remain at the original size?
#!/bin/bash
# Description: Verify the size change in CSS files

# Test: Search for nodeIcons size in CSS files
rg --type css -i 'nodeIcons.*(width|height)'

47-47: Ensure consistency in line width changes.

The width of the first two nodeLines has been reduced from 4px to 1px, consistent with the earlier change to childNodeLines.

Please confirm the following:

  1. Is this change intentional and consistent with the updated design for all line elements?
  2. Should the third nodeLine (if it exists) also have its width adjusted?
  3. Are there any visual or functional implications of having thinner lines throughout the wizard?
#!/bin/bash
# Description: Verify the consistency of line width changes

# Test: Search for line width in CSS files
rg --type css -i '(nodeLine|childNodeLine).*width'

42-42: Verify the color change for the last childNodeLine.

The background color of the third childNodeLine has been changed from transparent (rgba(0, 0, 0, 0)) to a dark gray/black color (rgb(25, 25, 25)). This change makes the last line visible instead of transparent.

Please confirm that this change is intentional and aligns with the updated design. Consider the following:

  1. Is this change supposed to highlight the last step in the wizard?
  2. Should this color be consistent with other dark elements in the UI?
packages/theme/src/wizard/vars.less (1)

13-13: Method name updated appropriately.

The method name has been changed from .component-css-vars-wizard() to .inject-Wizard-vars(), reflecting the new naming convention and improving clarity.

Comment on lines +15 to +131
--tv-Wizard-vertical-line-width: var(--tv-border-width, 1px);
// 垂直模式线条高度
--ti-wizard-vertical-line-height: var(--ti-common-line-height-8, 60px);
--tv-Wizard-vertical-line-height: 60px;
// 垂直模式图标定位模式(hide)
--ti-wizard-vertical-icon-position: absolute;
--tv-Wizard-vertical-icon-position: absolute;
// 垂直模式文本外边距(hide)
--tv-Wizard-vertical-line-position-top: 54px;
// 垂直模式文件外边距(hide)
--ti-wizard-vertical-position-top: -5px;
// 垂直模式文件外边距(hide)
--ti-wizard-vertical-line-position-top: 54px;
// 垂直模式文件外边距(hide)
--ti-wizard-vertical-icon-position-top: -40px;
--tv-Wizard-vertical-icon-position-top: -40px;
// 垂直模式节点部分整体高度
--ti-wizard-vertical-node-box-height: calc(var(--ti-wizard-vertical-line-height) + var(--ti-wizard-node-size) + 16px);
--tv-Wizard-vertical-node-box-height: calc(var(--tv-Wizard-vertical-line-height) + var(--tv-Wizard-node-size) + 16px);
// 垂直模式节点文字左外边距
--ti-wizard-vertical-text-margin-left: 21.5px;
--tv-Wizard-vertical-text-margin-left: 21.5px;
// 垂直模式节点描述文字上外边距
--ti-wizard-vertical-desc-margin-top: var(--ti-common-space-0, 0px);
--tv-Wizard-vertical-desc-margin-top: 0px;
// 垂直模式节点标题文字行高
--ti-wizard-vertical-title-line-height: var(--ti-common-line-height-number, 1.5);
--tv-Wizard-vertical-title-line-height: var(--tv-line-height-number, 1.5);
// 垂直模式节点标题字号
--ti-wizard-vertical-title-font-size: var(--ti-common-font-size-1, 14px);
--tv-Wizard-vertical-title-font-size: var(--tv-font-size-md, 14px);
// 垂直模式节点描述字号
--ti-wizard-vertical-desc-font-size: var(--ti-common-font-size-0, 12px);
--tv-Wizard-vertical-desc-font-size: var(--tv-font-size-sm, 12px);
// 垂直模式节点描述文本色
--ti-wizard-vertical-desc-text-color: var(--ti-common-color-text-weaken, #808080);
--tv-Wizard-vertical-desc-text-color: var(--tv-color-text-weaken, #808080);
// 垂直模式未完成节点描述文本色
--ti-wizard-vertical-wait-desc-text-color: var(--ti-common-color-bg-emphasize, #191919);
--tv-Wizard-vertical-wait-desc-text-color: var(--tv-color-text, #191919);
// 时间线节点尺寸
--ti-wizard-time-node-size: var(--ti-common-font-size-6, 32px);
--tv-Wizard-time-node-size: var(--tv-font-size-heading-xl, 32px);
// 时间线节点边框厚度
--ti-wizard-time-node-border-weight: 0;
--tv-Wizard-time-node-border-weight: 0;
// 时间线节点内图标宽度
--ti-wizard-time-node-icon-width: var(--ti-common-font-size-6, 32px);
--tv-Wizard-time-node-icon-width: var(--tv-font-size-heading-xl, 32px);
// 时间线节点内图标高度
--ti-wizard-time-node-icon-height: var(--ti-common-font-size-6, 32px);
--tv-Wizard-time-node-icon-height: var(--tv-font-size-heading-xl, 32px);
// 时间线节点内图标色
--ti-wizard-time-node-icon-color: var(--ti-common-color-bg-emphasize, #191919);
--tv-Wizard-time-node-icon-color: var(--tv-color-icon-hover, #191919);
// 时间线节点内图标背景色
--ti-wizard-time-node-bg-color: var(--ti-common-color-light, #ffffff);
--tv-Wizard-time-node-bg-color: var(--tv-color-bg-secondary, #ffffff);
// 时间线节点左侧日期最小宽度
--ti-wizard-time-left-min-width: var(--ti-common-size-width-medium, 120px);
--tv-Wizard-time-left-min-width: 120px;
// 时间线节点左侧日期右外边距
--ti-wizard-time-left-margin-right: var(--ti-common-space-2x, 8px);
--tv-Wizard-time-left-margin-right: var(--tv-space-md, 8px);
// 时间线节点左侧日期top定位(hide)
--ti-wizard-time-left-position-top: -28px;
--tv-Wizard-time-left-position-top: -28px;
// 时间线节点左侧时间top定位(hide)
--ti-wizard-time-left-point-position-top: -15px;
--tv-Wizard-time-left-point-position-top: -15px;
// 时间线节点左侧展开图标右外边距
--ti-wizard-time-left-icon-margin-right: var(--ti-common-space-10, 10px);
--tv-Wizard-time-left-icon-margin-right: 10px;
// 时间线节点左侧展开图标色
--ti-wizard-time-left-icon-color: var(--ti-common-color-icon-normal, #808080);
--tv-Wizard-time-left-icon-color: var(--tv-color-icon, #808080);
// 时间线节点左侧日期图标左外边距
--ti-wizard-time-left-icon-margin-left: var(--ti-common-space-2x, 8px);
--tv-Wizard-time-left-icon-margin-left: var(--tv-space-md, 8px);
// 时间线节点左侧日期图标尺寸
--ti-wizard-time-left-icon-size: var(--ti-common-font-size-2, 16px);
--tv-Wizard-time-left-icon-size: var(--tv-font-size-heading-xs, 16px);
// 时间线节点左侧文字行高
--ti-wizard-time-left-line-height: var(--ti-common-line-height-number, 1.5);
--tv-Wizard-time-left-line-height: var(--tv-line-height-number, 1.5);
// 时间线节点小圆点尺寸
--ti-wizard-time-dot-size: var(--ti-common-size-3x, 12px);
--tv-Wizard-time-dot-size: var(--tv-font-size-sm, 12px);
// 时间线节点右侧文字对齐方式(hide)
--ti-wizard-time-right-text-align: left;
--tv-Wizard-time-right-text-align: left;
// 时间线节点右侧文字左外边距
--ti-wizard-time-right-margin-left: 15.5px;
--tv-Wizard-time-right-margin-left: 15.5px;
// 时间线节点右侧文字垂直内边距
--ti-wizard-time-right-padding-vertical: var(--ti-common-space-0, 0px);
--tv-Wizard-time-right-padding-vertical: 0px;
// 时间线节点右侧标题底部外边距
--ti-wizard-time-right-title-margin-bottom: var(--ti-common-space-0, 0px);
--tv-Wizard-time-right-title-margin-bottom: 0px;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Inconsistent capitalization in CSS variable names.

The CSS variable names use a mix of uppercase and lowercase letters in the Wizard component, such as --tv-Wizard-font-size. For consistency and to adhere to standard CSS naming conventions, it's recommended to use all lowercase letters with hyphens. Consider renaming variables to use lowercase, like --tv-wizard-font-size.

Apply the following pattern to update the variable names:

- --tv-Wizard-font-size: var(--tv-font-size-md, 14px);
+ --tv-wizard-font-size: var(--tv-font-size-md, 14px);

This change would apply to all instances where Wizard appears in the CSS variable names.

Committable suggestion was skipped due to low confidence.

@zzcr zzcr merged commit ade7165 into dev Oct 16, 2024
6 checks passed
@kagol kagol deleted the refactor/wizard-theme-20241015 branch November 4, 2024 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring (重构)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants