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

[VDG] Fix bad flyout behavior after theme switch #8343

Merged

Conversation

SuperJMN
Copy link
Contributor

@SuperJMN SuperJMN commented Jun 10, 2022

Theme switching from the SearchBar makes the flyout appear in the wrong position.

The problem is that the behavior is detached and reattached. This causes issues with visuals (as popup locating in the wrong place).

This PR is a quick fix for the bad behavior.
It could have been more elegant (reopen after reattaching of previously detached). Unfortunately, it seems it's more complex than it initially looked.

Anyways, the behavior after this PR is completely acceptable, in my opinion.

I wouldn't like to spend too much time on this, because I would like to fully rewrite the popup behavior logic.

Fixes #8321

The flyout was in the wrong position.
@SuperJMN SuperJMN marked this pull request as draft June 10, 2022 11:20
@SuperJMN SuperJMN marked this pull request as ready for review June 10, 2022 11:28
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

tACK

This fixes #8321

@soosr
Copy link
Contributor

soosr commented Jun 11, 2022

I am not really happy with these focus losses, and I think @yahiheb was right here #7897 (comment) in the sense it is not the best solution.

SearchBar now breaks the usability for only keyboard users... When you keep pressing tabs and the focus reached the SearchBar, the next tabulator press breaks the focus and the user has to click somewhere in the software to give back the focus in order to continue using it with only the keyboard.

Anyway using the software with a keyboard has never been perfect... but this is something that we should consider for the future.

@SuperJMN
Copy link
Contributor Author

SuperJMN commented Jun 13, 2022

I am not really happy with these focus losses, and I think @yahiheb was right here #7897 (comment) in the sense it is not the best solution.

SearchBar now breaks the usability for only keyboard users... When you keep pressing tabs and the focus reached the SearchBar, the next tabulator press breaks the focus and the user has to click somewhere in the software to give back the focus in order to continue using it with only the keyboard.

Anyway using the software with a keyboard has never been perfect... but this is something that we should consider for the future.

Me neither, but I think it's still acceptable. (At least while we rewrite the SearchBarBehavior)
There are still edge cases that aren't handled correctly. Popups are sometimes unpredictable, and changing theme makes it worse.

Copy link
Contributor

@Szpoti Szpoti left a comment

Choose a reason for hiding this comment

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

tACK, it's a fix afterall, later it can be improved.

@soosr soosr merged commit d59c18e into WalletWasabi:master Jun 28, 2022
@SuperJMN SuperJMN deleted the fixes/8321-wrong-searchbar-flyout-position branch June 28, 2022 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SearchBar - flyout is in the wrong position
4 participants