Close popover on text field tap #1384
Conversation
|
@richardshiue , would you like to help review? This is a fix for your issue |
annieappflowy
added
grid
Sure, I'll review when I'm free |
|
Hello @Cyrine-benabid, thanks for contributing and tackling this bug! Your fix should close the popover correctly, but it's only going to happen by clicking the text field. This means that after I click on "Add option", the unwanted popover wouldn't disappear until I specifically click on the text field, even though the text field already has focus. A better solution would be to listen for focus changes in the popover's elements. I will try and lay down some steps you can follow to finish this task. |
frontend/app_flowy/lib/plugins/grid/presentation/widgets/common/text_field.dart
Show resolved
Hide resolved
Cyrine-benabid
force-pushed
the
fix-option_button_doesnt-close_popover
branch
2 times, most recently
from
65ce212
to
7c99d08
Compare
Cyrine-benabid
force-pushed
the
fix-option_button_doesnt-close_popover
branch
from
7c99d08
to
b7f369b
Compare
|
Hi @richardshiue ! Thank you for your review and your suggestion ! You can see in the video bellow my idea after the new commits. |
|
Also @richardshiue, this was my original implementation (forgot to push one commit), but I also have a working implementation of your proposition in another commit. Let me know if you think we should use your proposition instead, and I'll push this commit. |
|
Hello @Cyrine-benabid, thanks for the quick update. It solves the bug all right, no doubts there. But doing it the way I outlined above would also allow listening to the popover mutex to unrequest focus from the text field. I don't know that not supporting this will affect the UX much right now, if at all; but could be important if AppFlowy wants to improve keyboard accessibility in the future. I think it's the perfect time to ask for @appflowy's views. Apart from the above concern, LGTM! |
|
Hi @richardshiue, now I understand your point, thank you ! I pushed the implementation you suggested. |
Cyrine-benabid
force-pushed
the
fix-option_button_doesnt-close_popover
branch
from
74af374
to
1a53a0e
Compare
frontend/app_flowy/packages/flowy_infra_ui/lib/widget/rounded_input_field.dart
Outdated
Show resolved
Hide resolved
frontend/app_flowy/lib/plugins/grid/presentation/widgets/header/type_option/select_option.dart
Outdated
Show resolved
Hide resolved
frontend/app_flowy/lib/plugins/grid/presentation/widgets/header/type_option/select_option.dart
Outdated
Show resolved
Hide resolved
frontend/app_flowy/lib/plugins/grid/presentation/widgets/header/type_option/select_option.dart
Outdated
Show resolved
Hide resolved
frontend/app_flowy/lib/plugins/grid/presentation/widgets/header/type_option/select_option.dart
Outdated
Show resolved
Hide resolved
|
Almost there! |
|
LGTM! Thanks for your work and patience with me during this process. Just need AppFlowy team's approval now. |
|
Thanks to you @richardshiue for guiding me ! Looking forward to contribute more on this project 🙌 ! Hopefully this PR can be merged or hacktoberfest-accepted today 🙏 |
|
Great work @Cyrine-benabid @richardshiue ! |
fixes #1368