docs: fix back to top does not work and the document tab title does n… #2712
Conversation
…ot sticky after the layout is modified
WalkthroughThe pull request focuses on correcting and improving the scrolling behavior and layout in Vue components. The main changes involve fixing a typographical error in the Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant ScrollElement
User->>Component: Interact with page
Component->>ScrollElement: Set scroll listener
ScrollElement-->>Component: Track scroll position
Component->>Component: Update back-to-top button visibility
User->>Component: Click back-to-top
Component->>ScrollElement: Scroll to top
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
Walkthrough此PR修复了在布局修改后，返回顶部功能失效以及文档页签标题未吸顶的问题。主要通过修正元素ID和调整CSS样式来实现。 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| document.getElementById('doc-layout-scoller').scrollTo({ | ||
| top: scrollTarget.offsetTop + 52, | ||
| document.getElementById('doc-layout-scroller').scrollTo({ | ||
| top: scrollTarget.offsetTop, |
There was a problem hiding this comment.
The removal of + 52 from scrollTarget.offsetTop may affect the scroll position. Ensure that this change does not negatively impact the intended scroll behavior.
WalkthroughThis PR fixes the problem that after the layout is modified, the return to top function fails and the document tab title does not move to the top. This is mainly achieved by correcting element IDs and adjusting CSS styles. Changes
|
|
[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: 0
🧹 Nitpick comments (4)
examples/sites/src/views/components/components.vue (4)
236-236: Check if offset-top of 56 is suitable
You hard-coded:offset-top="56". Consider a dynamic approach if the header height is not always 56px.
488-489: Use smooth scrolling for hash navigation
UsingscrollTarget.offsetTopandbehavior: 'smooth'is user-friendly. Consider a fallback if older browsers need support.
503-503: Repeated scroll to top call
Same approach as line 473. If more lines repeat this pattern, consider extracting a helper function for maintainability.
806-806: Switched from padding-top to margin-top
Altering the layout from padding to margin can affect container positioning. Verify that it doesn’t unintentionally break existing CSS flows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/sites/src/views/components/components.vue(8 hunks)examples/sites/src/views/components/float-settings.vue(3 hunks)
🔇 Additional comments (11)
examples/sites/src/views/components/float-settings.vue (3)
152-152: Good job referencing the correct element for smooth scrolling.
You’ve correctly updated the target element ID and used the behavior: 'smooth' option. Note that not all legacy browsers support smooth scrolling by default.
199-199: Verify docLayout not null before assigning
You are already checking if (docLayout) before assigning the scroll handler. This ensures no runtime errors if the element is not found.
221-221: Ensure consistent cleanup of the scroll listener
Reassigning onscroll to null is okay, but confirm that other potential listeners or references are not left dangling if multiple assignments exist. If only this assignment is used, this is fine.
examples/sites/src/views/components/components.vue (8)
19-19: Corrected element ID for the scroller container
Changing the ID to doc-layout-scroller aligns with references in related files and ensures a uniform approach to scrolling logic.
241-241: Consistent container-id usage
Pointing container-id="#doc-layout-scroller" matches the layout changes, ensuring the anchor scrolls within the intended container.
473-473: Smoothly scroll to top
This logic mirrors the fix in float-settings.vue. Good consistency in referencing doc-layout-scroller.
486-486: Skip scroll when running tests
Your condition checks !isRunningTest before scrolling. This should help keep automated tests stable.
624-629: Runtime checks for null
You reference docLayout and derive layout measurements before checking. Ensure docLayout is not null to avoid potential errors on edge cases.
635-635: Add event listener to doc-layout-scroller
This matches the usage in float-settings.vue. Good job unifying the scroll event element.
643-643: Remove event listener on unmount
Cleanly removing the scroll event in onUnmounted is a best practice, preventing memory leaks.
830-830: Sticky tab header
Setting top: 0 ensures the header remains in view. Confirm that overlapping elements at higher z-indices do not interfere with the sticky header.
…ot sticky after the layout is modified
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
Bug Fixes
doc-layout-scrollerelement ID across multiple componentsStyle
.docs-contentclass