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

feat: The Activity now notifies when the drawer status has changed #4560

Merged
merged 2 commits into from
Apr 4, 2022

Conversation

g123k
Copy link
Contributor

@g123k g123k commented Mar 20, 2022

To fix #4540 and more particularly the issue with the focus, I had to make some changes.
Right now, the MainActivity has the following lifecycle:

  • [User] Click to open the drawer
  • [Activity] Close the keyboard
  • [User] Select of choice
  • [Activity] Change the fragment and close the keyboard

The problem here is that the last keyboard event happens after the fragment is added/attached (after the onResume).

To fix this, the Fragments now can listen to drawer events.
Here is the new lifecycle:

  • [User] Click to open the drawer
  • [Activity] Close the keyboard
  • [User] Select of choice
  • [Activity] Change the fragment, close the keyboard and notify the attached Fragment
  • [Fragment] Can force the keyboard to be reopened

@g123k g123k requested a review from a team as a code owner March 20, 2022 14:13
@teolemon teolemon requested a review from VaiTon March 21, 2022 08:32
@teolemon
Copy link
Member

@g123k small merge conflicts to solve

@teolemon
Copy link
Member

@g123k should I keep both imports ?

@g123k
Copy link
Contributor Author

g123k commented Mar 26, 2022

@g123k should I keep both imports ?

Yes you can

@VaiTon
Copy link
Member

VaiTon commented Mar 27, 2022

@g123k we should specify that with this approach the activity can only have a fragment listening to the drawer at any given time. Maybe also implement a control on the onAttach to check if the listener was null before attaching.

@g123k
Copy link
Contributor Author

g123k commented Mar 28, 2022

Thanks for your feedback @VaiTon, I will add them

@g123k
Copy link
Contributor Author

g123k commented Mar 29, 2022

@VaiTon I've changed my implementation to allow multiple listeners (with a Set).
Is-it better with this approach?

@sonarcloud
Copy link

sonarcloud bot commented Mar 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@VaiTon
Copy link
Member

VaiTon commented Apr 4, 2022

Sorry for the delay!

@VaiTon VaiTon merged commit ef12188 into openfoodfacts:develop Apr 4, 2022
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.

UX problems on the search by text screen
3 participants