fix(site): fix Popper layers separate when scrolling through document #2502
Conversation
WalkthroughThe changes in this pull request involve significant modifications to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)packages/theme/src/base/reset.less (3)
The implementation provides a uniform 4px margin for all placement directions, which helps maintain visual consistency across different components (select, userlink, roles, dept). Line range hint The switch from border-based to div-based arrows is a good improvement:
The implementation aligns well with the PR objective of fixing popper layer separation during scrolling.
While consolidating styles is good for maintainability, the Consider adding a fallback for browsers that don't support .tiny-popconfirm-popover:has(.popper__arrow),
.tiny-popper:has(.popper__arrow),
.tiny-tooltip__popper:has(.popper__arrow) {
+ /* Fallback for browsers that don't support :has() */
+}
+
+/* Separate selectors as fallback */
+.tiny-popconfirm-popover .popper__arrow,
+.tiny-popper .popper__arrow,
+.tiny-tooltip__popper .popper__arrow {
/* Same styles as above */
}✅ Verification successfulBrowser compatibility concerns with :has() are valid, but the implementation is acceptable The usage of
The codebase shows a pattern of handling compatibility issues where needed (e.g., 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check if there are any browser compatibility fallbacks in the codebase
# Test: Look for any CSS fallback patterns or polyfills for :has()
rg -i "has\(\)" "packages/theme" -A 5 -B 5
Length of output: 42 Script: #!/bin/bash
# Let's check the entire reset.less file to understand the context better
cat packages/theme/src/base/reset.less
# Also check for any browser compatibility related configurations or comments
rg -i "browser|compat|support" packages/theme
Length of output: 4533 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 (
|
|
[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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
examples/sites/src/views/components/components.vue (1)
Typo found in scroll container ID: 'doc-layout-scoller' should be 'doc-layout-scroller'
The ID
doc-layout-scollerappears to be misspelled (missing 'r') and is used consistently but incorrectly throughout the components.vue file. This typo appears in:
- HTML element definition:
<div class="docs-content" id="doc-layout-scoller">- Container ID prop:
container-id="#doc-layout-scoller"- Multiple getElementById calls
- Comments
While the code works because the ID is consistently misspelled everywhere it's used, it should be corrected to
doc-layout-scrollerfor better maintainability and clarity.🔗 Analysis chain
Line range hint
476-494: LGTM: Scroll handling changes align with PR objectiveThe modifications to scroll handling functions correctly implement the new scrolling container, which should fix the Popper layers separation issue during document scrolling. However, remember to update the ID references after fixing the typo.
Let's verify all occurrences of the ID are updated:
Also applies to: 504-509, 625-630, 636-639, 644-647
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all occurrences of the ID to ensure consistent updates rg "doc-layout-scoller" --type vueLength of output: 62
Script:
#!/bin/bash # Search for the ID pattern without file type restriction since .vue files might not be recognized rg "doc-layout-scoller"Length of output: 1096
Script:
#!/bin/bash # Use find and grep as a fallback to search for the pattern find . -type f -exec grep -l "doc-layout-scoller" {} \;Length of output: 111
Script:
#!/bin/bash # Search for similar ID patterns to check if there might be a typo rg "doc-layout-scoll" rg "doc-layout-scroll"Length of output: 1117
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
examples/sites/src/views/components/components.vue(8 hunks)examples/sites/src/views/layout/layout.vue(0 hunks)
💤 Files with no reviewable changes (1)
- examples/sites/src/views/layout/layout.vue
🔇 Additional comments (1)
examples/sites/src/views/components/components.vue (1)
812-814: LGTM: Appropriate overflow handling for scroll container
The overflow properties are correctly set to enable vertical scrolling while preventing horizontal overflow. The transition property ensures smooth visual changes.
| @@ -16,7 +16,7 @@ | |||
| </div> | |||
| <span class="docs-header-spacer"></span> | |||
| </header> | |||
| <div class="docs-content"> | |||
| <div class="docs-content" id="doc-layout-scoller"> | |||
There was a problem hiding this comment.
Fix typo in ID: doc-layout-scoller should be doc-layout-scroller
The ID has a typo: "scoller" should be "scroller". This needs to be fixed consistently throughout the file to maintain clarity and prevent future maintenance issues.
- <div class="docs-content" id="doc-layout-scoller">
+ <div class="docs-content" id="doc-layout-scroller">
- container-id="#doc-layout-scoller"
+ container-id="#doc-layout-scroller"Also applies to: 244-244
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
修复弹出层分离的问题。 调整组件页面的滚动元素， 改为 #doc-layout-scroller
以后整改时，可以通过 $refs 去访问 #doc-layout-scroller元素了
统一tiny-popper的样式
Summary by CodeRabbit
New Features
Bug Fixes
Style