Page MenuHomePhabricator

[Cashtab] Improve scrolling and header minification
ClosedPublic

Authored by bytesofman on Fri, Apr 19, 23:17.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC6d4047b7314c: [Cashtab] Improve scrolling and header minification
Summary

This still has not been quite right. Improve implementation so that it is actually getting expected behavior (i.e., header is minified when user scrolls past 63 px, header is not un-minified until user scrolls back above 15px)

Note: react router not resetting scroll to 0 on nav is expected behavior. It can be fixed with this, but we need to migrate the routers first. https://reactrouter.com/en/main/components/scroll-restoration

Note: I tried to add integration tests for the behavior, however react testing library uses JSDOM which, apparently, does not know anything about scrolling. We would need to use a browser-based testing library. We may need to do this eventually anyway to properly test the browser extension. But we can't do it today.

Test Plan

npm test, npm start and better behavior on receive and send screens (screens that do not usually have any scrolling).

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman edited the summary of this revision. (Show Details)
emack requested changes to this revision.Sat, Apr 20, 07:51
emack added a subscriber: emack.

Tested fine, just a nit on code pattern

cashtab/src/components/App/App.js
113–117 ↗(On Diff #47334)
This revision now requires changes to proceed.Sat, Apr 20, 07:51
bytesofman marked an inline comment as done.

better code structure, comments on behavior

cashtab/src/components/App/App.js
113–124 ↗(On Diff #47336)

we could probably abbreviate this further, but imo this structure is good for code readabillity. It is not immediately clear what behavior we are trying to achieve through a simplified if statement...part of the reason for this diff is that my last attempt at using a one-liner was incorrect and not doing what I thought it was doing.

113–117 ↗(On Diff #47334)

this is not the same behavior -- now we are no longer considering UNPIN_MINIFIED_WALLET_MENU_SCROLLY

Updated code comments and improved syntax

This revision is now accepted and ready to land.Sat, Apr 20, 12:06