fix(color-picker): auto flip#4187
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughRefactors the color picker to use a Popover component (manual trigger) instead of a Transition wrapper, updates component markup and panel sizing, adds a popover dependency, and adjusts Less styles for popper/panel positioning and appearance. Changes
Sequence DiagramsequenceDiagram
actor User
participant CP as ColorPicker (pc.vue)
participant PO as Popover (tiny-popover)
participant CSP as ColorSelectPanel
User->>CP: Click trigger
CP->>CP: set state.isShow = true
CP->>PO: v-model update (show)
PO->>PO: compute popper position
PO->>CSP: mount/render panel (width: 330px)
CSP->>User: display panel
User->>CSP: choose color
CSP->>CP: emit color value
CP->>CP: set state.isShow = false
CP->>PO: v-model update (hide)
PO->>User: close popover
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/vue/src/color-picker/package.json (1)
12-12: LGTM — dependency addition is correct.
@opentiny/vue-popoverexists at the same workspace version (3.30.0) and exports a default component, matching the newimport TinyPopover from '@opentiny/vue-popover'inpc.vue.Minor nit: other dependencies here use
workspace:~while this one usesworkspace:^. Consider aligning toworkspace:~for consistency with the rest of the block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue/src/color-picker/package.json` at line 12, The dependency entry for "@opentiny/vue-popover" in package.json uses "workspace:^" which is inconsistent with the other dependencies that use "workspace:~"; update the version specifier for "@opentiny/vue-popover" to "workspace:~" to match the project's dependency style (this affects the package.json dependency entry referencing "@opentiny/vue-popover" and is referenced by the import TinyPopover in pc.vue).packages/vue/src/color-picker/src/pc.vue (1)
38-40: Remove commented-out<Transition>block.Leftover dead code from the previous implementation — the popover now owns the enter/leave transition. Please delete rather than leaving it commented.
🧹 Proposed cleanup
- <!-- <Transition name="tiny-zoom-in-top"> - - </Transition> --> </tiny-popover>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue/src/color-picker/src/pc.vue` around lines 38 - 40, Remove the leftover commented-out <Transition> block in pc.vue: delete the entire commented section <!-- <Transition name="tiny-zoom-in-top"> ... </Transition> --> so the file no longer contains dead commented transition code (the popover now manages transitions). Ensure no remaining commented Transition lines remain in pc.vue and run formatting/linting to keep the file tidy.packages/theme/src/color-picker/index.less (1)
54-57: Prefer specificity over!importantwhere possible.Both declarations use
!importantto override the default.tiny-popoverstyling. SincepopperClassis applied on the same element as.tiny-popover(seepackages/vue/src/popover/src/pc.vueline ~6), a selector like&.@{color-picker-prefix-cls}__poppercombined with the parent popover class, or simply relying on source-order cascade with equal specificity, is usually enough.!importantmakes future overrides (e.g. user theming) harder.Not a blocker — consider as a follow-up cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/theme/src/color-picker/index.less` around lines 54 - 57, Replace the two !important overrides in the color picker popper rule by increasing selector specificity instead: target the same element that gets both the popperClass and .tiny-popover (e.g., combine the popper parent/popover class with &__popper or use &.@{color-picker-prefix-cls}__popper) so padding:0 and background:transparent win via specificity or source order rather than using !important; update the selector where &__popper is defined (color-picker's LESS) and remove the !important flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/theme/src/color-picker/index.less`:
- Around line 54-57: Replace the two !important overrides in the color picker
popper rule by increasing selector specificity instead: target the same element
that gets both the popperClass and .tiny-popover (e.g., combine the popper
parent/popover class with &__popper or use &.@{color-picker-prefix-cls}__popper)
so padding:0 and background:transparent win via specificity or source order
rather than using !important; update the selector where &__popper is defined
(color-picker's LESS) and remove the !important flags.
In `@packages/vue/src/color-picker/package.json`:
- Line 12: The dependency entry for "@opentiny/vue-popover" in package.json uses
"workspace:^" which is inconsistent with the other dependencies that use
"workspace:~"; update the version specifier for "@opentiny/vue-popover" to
"workspace:~" to match the project's dependency style (this affects the
package.json dependency entry referencing "@opentiny/vue-popover" and is
referenced by the import TinyPopover in pc.vue).
In `@packages/vue/src/color-picker/src/pc.vue`:
- Around line 38-40: Remove the leftover commented-out <Transition> block in
pc.vue: delete the entire commented section <!-- <Transition
name="tiny-zoom-in-top"> ... </Transition> --> so the file no longer contains
dead commented transition code (the popover now manages transitions). Ensure no
remaining commented Transition lines remain in pc.vue and run formatting/linting
to keep the file tidy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2a6d27f2-8671-4d85-93cb-f8266cd45648
📒 Files selected for processing (4)
packages/theme/src/color-picker/index.lesspackages/theme/src/color-select-panel/index.lesspackages/vue/src/color-picker/package.jsonpackages/vue/src/color-picker/src/pc.vue
💤 Files with no reviewable changes (1)
- packages/theme/src/color-select-panel/index.less
2 participants
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: #4058
What is the new behavior?
Does this PR introduce a breaking change?
Other information
#4088
很长时间没有更新，所以就开了这个PR
Summary by CodeRabbit
Refactor
Style
Chores
Tests