Skip to content

feat: scrolls page to top on sidebar Home click#14936

Merged
claudiiafg merged 1 commit intodevelopfrom
feat/LIVE-27082_scroll
Mar 3, 2026
Merged

feat: scrolls page to top on sidebar Home click#14936
claudiiafg merged 1 commit intodevelopfrom
feat/LIVE-27082_scroll

Conversation

@claudiiafg
Copy link
Contributor

@claudiiafg claudiiafg commented Mar 2, 2026

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

📝 Description

This pull request introduces a minor feature to improve the user experience in the Ledger Live Desktop app. Now, when a user clicks "Home" in the sidebar while already on the home page, the app will smoothly scroll the page to the top. This is achieved through a custom event and coordinated updates to the sidebar and page view models.

Screen.Recording.2026-03-02.at.16.13.10.mov

❓ Context

LIVE-27082


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@live-github-bot live-github-bot bot added the desktop Has changes in LLD label Mar 2, 2026
@claudiiafg claudiiafg marked this pull request as ready for review March 2, 2026 16:38
@claudiiafg claudiiafg requested a review from a team as a code owner March 2, 2026 16:38
Copilot AI review requested due to automatic review settings March 2, 2026 16:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a small UX improvement in Ledger Live Desktop’s MVVM layout so that clicking Home in the sidebar while already on the Home route triggers a smooth scroll back to the top of the page scroller.

Changes:

  • Introduces a shared SCROLL_TO_TOP_EVENT constant for cross-component scroll-to-top coordination.
  • Dispatches the scroll-to-top event from the sidebar when Home is clicked while already on Home (Wallet 4.0 main nav).
  • Listens for the event in the Page view model and scrolls the page scroller to the top.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
apps/ledger-live-desktop/src/mvvm/components/SideBar/useSideBarViewModel.ts Dispatches a scroll-to-top custom event when Home is clicked while already on Home (Wallet 4.0 main nav).
apps/ledger-live-desktop/src/mvvm/components/Page/usePageViewModel.ts Adds a window event listener that scrolls the page scroller to top when the custom event is received.
apps/ledger-live-desktop/src/mvvm/components/Page/constants.ts Defines the SCROLL_TO_TOP_EVENT string constant.
.changeset/late-crabs-prove.md Declares a Desktop changeset for the new UX behavior.

value === "home" &&
location.pathname === SIDEBAR_VALUE_TO_PATH.home
) {
window.dispatchEvent(new CustomEvent(SCROLL_TO_TOP_EVENT));
Copy link
Member

Choose a reason for hiding this comment

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

Let's create a function like "handleScrollToTop" to make it more explicit, rather than a window.dispatchEvent wdyt?

const onClickScrollUp = useCallback(() => scrollToTop(), [scrollToTop]);

// When sidebar (or elsewhere) dispatches SCROLL_TO_TOP_EVENT, scroll the page scroller to top
useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

useLayoutEffect could be more appropriated no?

@claudiiafg claudiiafg force-pushed the feat/LIVE-27082_scroll branch from 1e1474c to 56c4b59 Compare March 2, 2026 18:02
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

⚠️ E2E tests are required

Changes detected require e2e testing before merge (even before asking for any review).

🖥️ Desktop

-> Run Desktop E2E

  • Select "Run workflow"
  • Branch: feat/LIVE-27082_scroll
  • Device: nanoSP or stax

Copilot AI review requested due to automatic review settings March 2, 2026 18:05
@claudiiafg claudiiafg force-pushed the feat/LIVE-27082_scroll branch from 56c4b59 to 80e7400 Compare March 2, 2026 18:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 3, 2026

@claudiiafg claudiiafg merged commit cf973c9 into develop Mar 3, 2026
99 of 103 checks passed
@claudiiafg claudiiafg deleted the feat/LIVE-27082_scroll branch March 3, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

desktop Has changes in LLD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants