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

fix: weird scroll behaviour in AccountTypeNetworkSearch #1767

Merged
merged 8 commits into from
Dec 30, 2024
Merged

Conversation

alecdwm
Copy link
Member

@alecdwm alecdwm commented Dec 24, 2024

I originally used @tanstack/react-virtual instead of the built-in virtual prop of <Combobox /> because I needed the Network | Account Type header inside of the dropdown list.

This PR implements an alternative solution; it uses the built-in virtual prop instead of @tanstack/react-virtual, and then implements the header as a faux <ComboboxOption />.

As far as I can tell this fixes the weird scrolling behaviour.

@alecdwm alecdwm added the bug A bug label Dec 24, 2024
@alecdwm alecdwm requested review from chidg and UrbanWill December 24, 2024 21:11
@alecdwm alecdwm self-assigned this Dec 24, 2024
@alecdwm alecdwm enabled auto-merge (squash) December 24, 2024 21:12
* feat: add account redesign

* fix: deselect AccountTypeNetworkSearch on click

* fix: optional AccountCreateMethodButton.networks prop

* fix: include nativetoken symbol in network search

* fix: copy feedback

* fix: also disable account type picker on second add/watch account page

* fix: navigate to correct tab when clicking on `Add Account` breadcrumb

* fix: copy feedback

* feat: add back button

* fix: watched account form spacing

* fix: trim account type network search input

* fix: account type search field font size
@alecdwm alecdwm force-pushed the fix/weird-scrolling branch from 81f7fe7 to c478796 Compare December 29, 2024 02:34
@alecdwm alecdwm disabled auto-merge December 29, 2024 02:35
UrbanWill
UrbanWill previously approved these changes Dec 30, 2024
Copy link
Contributor

@UrbanWill UrbanWill left a comment

Choose a reason for hiding this comment

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

LGTM! 🔥

Pushed to minor fixes:

1- Dropdown had a fixed size, when the search result has fewer options the dropdown should be smaller.

Screen shot before the fix:
Screenshot 2024-12-30 at 10 13 08

2- Searching and then hitting enter caused the app to crash because an empty option is still a default combo box option. Created a handleChange fn for that case, and tidied up side effects inside this fn as well.

Example before the fix:

Screen.Recording.2024-12-30.at.10.37.35.mov

Copy link
Contributor

@chidg chidg left a comment

Choose a reason for hiding this comment

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

LGTM!

@chidg chidg merged commit 88ed218 into dev Dec 30, 2024
6 checks passed
@chidg chidg deleted the fix/weird-scrolling branch December 30, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants