Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser: bug fixes, animation and UI improvements #5618

Merged
merged 18 commits into from
Apr 11, 2024

Conversation

christianbaroni
Copy link
Member

@christianbaroni christianbaroni commented Apr 11, 2024

What changed (plus any additional context for devs)

  • Fixes homepage scrolling
  • Fixes the screenshot flash that would sometimes happen when switching the active tab, by tying the screenshot display logic to the JS state that drives the WebView freezing
  • Adds an extra guard to prevent currentlyOpenTabIds and tabStates from becoming out of sync, which I noticed happen a couple times when quickly closing tabs by tapping the X on each tab
    • The underlying problem that causes them to become out of sync is that the operation queue is unblocked after tabStates is set, rather than after the value of tabStates updates. However, I found it to be slow and unreliable unblocking the queue by listening for changes in tabStates. I’m sure there are better solutions, but for now I added an additional step to processOperationQueueWorklet that strips out any tabs in the updated tabStates object that don't at that moment exist in currentlyOpenTabIds. This should reliably fix all sync issues caused by quickly closing tabs, essentially by resolving any out-of-sync states that do occur on the fly, before they compound and become a problem.
  • Makes it harder to miss when tapping the tab close X buttons
  • Adds closeAllTabsWorklet, which we’ll want to use in the near future to allow closing all tabs
  • Improves favorites URL standardization
  • Improves website logo detection by covering more icon types and searching for the highest-res icon (currently stops searching at >= 180px or if an SVG is found)
    • The script is a little more complex now but it doesn’t seem to have any noticeable performance impact — I tried to keep it as efficient as possible
  • Consolidates the website metadata into a single message
  • Makes the tab view ScrollView height animate when the number of tab rows changes, which may also slightly improve performance because it removes the inline ScrollView height setting
  • Adds an invisible autoscroll to vertically center the active tab in the tab view
    • As soon as a tab becomes fully open, the tab view is invisibly scrolled to ensure that if the tab view is re-entered, the active tab is centered vertically on the screen (or as close as possible if the tab is near the top or bottom of the list)
  • Fixes a crash I noticed coming from AccountIcon, where in getActiveSession(), activeSessionAddress was defined but sessions was not
  • Some minor misc. cleanup and tab operation type improvements

Screen recordings / screenshots

What to test

src/components/DappBrowser/Homepage.tsx Outdated Show resolved Hide resolved
Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

LGTM

src/components/DappBrowser/Homepage.tsx Outdated Show resolved Hide resolved
@christianbaroni christianbaroni merged commit 328cbcd into develop Apr 11, 2024
6 checks passed
@christianbaroni christianbaroni deleted the @christian/queue-improvements branch April 11, 2024 16:41
BrodyHughes added a commit that referenced this pull request Apr 17, 2024
…ecks-language

* 'develop' of github.com:rainbow-me/rainbow: (100 commits)
  [WIP]: Swaps v2 quote fetching (#5601)
  chore: app start up spring cleaning (#5622)
  fix remote config (#5627)
  would it kill you to log this only once (#5626)
  Recents (#5625)
  wc: improvements (#5616)
  Degen chain support (#5621)
  send: check contract address (#5586)
  tx requests: metadata (#5584)
  audit: phin (#5624)
  Fix: Wallets being marked as backed up by walletLoadState() (#5593)
  NFTs: filter instead of throw error when NFT has invalid network (#5537)
  Dapp Browser: Search (#5617)
  Browser: bug fixes, animation and UI improvements (#5618)
  cleanup file imports and duplicate types (#5619)
  requests: generalize analytics (#5589)
  browser: account icon clean up (#5612)
  Fix no tab and links (#5613)
  more e2e changes (#5558)
  . (#5615)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants