-
Notifications
You must be signed in to change notification settings - Fork 299
fix(site): fix source code's highlight color #2968
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
WalkthroughThis pull request updates several Vue components and theme styles. The component changes primarily refactor CSS styles to utilize CSS variables for text colors and icon theming. The demo component now employs a directive (v-auto-tip) in place of multiple tooltip components for streamlined tooltip functionality. Additionally, the theme files have been modified to introduce a new CSS variable and update tooltip color references, ensuring visual consistency across light and dark themes. No public API or exported entity declarations have been altered. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
examples/sites/src/views/components-doc/components/async-highlight.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/src/views/components-doc/components/contributor.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/src/views/components-doc/components/demo.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis PR addresses the issue of source code highlight color in the project. It involves updating various components and styles to ensure consistent color theming using CSS variables. The changes include modifying color values and replacing hardcoded colors with theme variables. Changes
|
> | ||
<i class="i-ti-playground ml8 h:c-success ti-w16 ti-h16 ti-cur-hand" @click="openPlayground(demo)" /> | ||
</tiny-tooltip> | ||
<i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of tiny-tooltip
and replacement with v-auto-tip
should be carefully tested to ensure that the new directive behaves as expected in all scenarios, especially regarding tooltip positioning and display logic.
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this 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
🧹 Nitpick comments (1)
examples/sites/src/views/components-doc/components/contributor.vue (1)
59-61
: Consider using theme variable for tooltip text color.While you've updated the title's color to use a theme variable, the tooltip text color is still hardcoded as
#191919
. For better theme consistency, consider using the appropriate CSS variable here as well..cmp-contributor-tip { font-size: 14px; - color: #191919; + color: var(--tv-color-text-inverse-black); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/sites/src/assets/images/leftMenu/cmp-business-components.svg
is excluded by!**/*.svg
examples/sites/src/assets/images/leftMenu/directives-custom-instruction.svg
is excluded by!**/*.svg
📒 Files selected for processing (7)
examples/sites/src/views/components-doc/components/async-highlight.vue
(3 hunks)examples/sites/src/views/components-doc/components/contributor.vue
(1 hunks)examples/sites/src/views/components-doc/components/demo.vue
(4 hunks)examples/sites/src/views/components-doc/components/float-settings.vue
(3 hunks)packages/theme/src/base/dark-theme.less
(1 hunks)packages/theme/src/base/vars.less
(1 hunks)packages/theme/src/tooltip/vars.less
(1 hunks)
🔇 Additional comments (15)
packages/theme/src/base/vars.less (1)
303-303
: Good addition of theming variable for tooltip text.Adding the
--tv-color-text-inverse-black
variable for tooltip light text improves theme consistency and makes it easier to manage text colors across different themes.packages/theme/src/base/dark-theme.less (1)
312-312
: Good parallel implementation for dark theme.Adding the same variable
--tv-color-text-inverse-black
in the dark theme with appropriate value maintains consistency between themes. This ensures proper text color for tooltips in dark mode.examples/sites/src/views/components-doc/components/contributor.vue (1)
35-35
: Good use of theme variable for title color.Replacing hardcoded color with the CSS variable
var(--tv-color-text)
improves theme consistency across the application.packages/theme/src/tooltip/vars.less (1)
66-66
: Good update to tooltip text color variable.Updating the light tooltip text color to use the new
--tv-color-text-inverse-black
variable improves visual consistency across the application and helps fix the source code highlight color issue mentioned in the PR title.examples/sites/src/views/components-doc/components/async-highlight.vue (3)
71-71
: Good theming improvement!Adding the color variable
var(--tv-color-text)
ensures consistent text coloring in code previews across different themes. This change aligns with the PR objective to fix source code highlighting colors.
84-84
: Improved contrast for attribute highlighting.Changing the color from
#00c
torgb(20, 20, 219)
provides better visibility while maintaining a similar blue hue. The RGB format also offers more precision for this particular shade.
98-98
: Changed title highlight from blue to green.The title highlight color has been changed from blue (
#00f
) to green (#170
), which better differentiates titles from attributes in code snippets. This improves readability and visual hierarchy in the code preview.examples/sites/src/views/components-doc/components/float-settings.vue (4)
295-298
: Good icon theming implementation.Adding CSS variables for icon colors helps maintain consistent styling across themes. This change supports the theming system by using
var(--tv-color-icon)
instead of hardcoded values.
304-305
: Improved hover state with theming variables.Replacing hardcoded hover colors with the CSS variable
var(--tv-color-icon-hover)
ensures consistent interactive behavior across different themes.
341-341
: Consistent text theming.Changing the title color to use
var(--tv-color-text)
maintains consistency with other themed text elements and improves theme adaptability.
364-367
: Enhanced theme support for popover backgrounds.Adding theme-specific background colors using CSS variables ensures the popover looks consistent in both light and dark modes. This is especially important for UI elements that appear on top of other content.
examples/sites/src/views/components-doc/components/demo.vue (4)
76-76
: Updated imports to support the tooltip directive change.Correctly updated imports to replace the Tooltip component with the AutoTip directive, aligning with the implementation changes in the template.
340-341
: Improved dark theme container styling.Removing the border in dark mode provides a cleaner look that better aligns with dark theme design principles where borders are typically less pronounced.
346-348
: Enhanced light theme container styling.Adding a white background and border to the container improves visual definition in light mode, providing better separation from the surrounding content.
379-384
: Added themed icon styling.The new styling for icons ensures they inherit theme colors and provides appropriate hover behavior, improving accessibility and visual feedback. Using CSS variables ensures consistency with the overall theming system.
<i | ||
v-auto-tip="{ content: state.copyTip, effect: 'light', always: true }" | ||
:class="state.copyIcon" | ||
class="ti-w16 ti-h16 ti-cur-hand" | ||
@click="copyCode(demo)" | ||
@mouseout="resetTip()" | ||
/> | ||
<i | ||
v-auto-tip="{ | ||
content: demo.isOpen ? i18nByKey('hideCode') : i18nByKey('showCode'), | ||
effect: 'light', | ||
always: true | ||
}" | ||
:class="!!demo.isOpen ? 'i-ti-codeslash' : 'i-ti-code'" | ||
class="ti-ml8 ti-w16 ti-h16 ti-cur-hand" | ||
@click="toggleDemoCode(demo)" | ||
/> | ||
|
||
<i | ||
v-auto-tip="{ content: i18nByKey('playground'), effect: 'light', always: true }" | ||
class="i-ti-playground ml8 ti-w16 ti-h16 ti-cur-hand" | ||
@click="openPlayground(demo)" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improved tooltip implementation with v-auto-tip directive.
Replacing multiple <tiny-tooltip>
components with <i>
elements using the v-auto-tip
directive simplifies the markup and likely improves performance. The directive-based approach is more concise while maintaining the same functionality.
PR
修复源码的高亮
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Style
Refactor