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

fixed shortcut issue #3959

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

shashank-0-0
Copy link
Contributor

@shashank-0-0 shashank-0-0 commented Jul 27, 2024

Fixes #3957

The issue was with navigateTo(), which breaks the back stack (https://stackoverflow.com/questions/71525493/bottom-navigation-bar-malfunctions-when-navigating-from-a-fragment)
so used the NavigationUI API to navigate.

Screenshots

Record_2024-07-27-18-52-11.mp4

ready to rework if changes are needed.

@kelson42
Copy link
Collaborator

@MohitMaliDeveloper What is the status here?

Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

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

@shashank-0-0 Thank you for the PR, can you please also fix the GET_CONTENT shortcut not working when the application is in the background? It works when the application is freshly opened, but not if I open it when the application is in background.

You need handle this intent in the onNewIntent method in KiwixMainActivity. Also can you please write the automated test case for this fix so that we can avoid this type of errors in future. If you need any kind of help in testing let me know.

@shashank-0-0
Copy link
Contributor Author

@shashank-0-0 Thank you for the PR, can you please also fix the GET_CONTENT shortcut not working when the application is in the background? It works when the application is freshly opened, but not if I open it when the application is in background.

You need handle this intent in the onNewIntent method in KiwixMainActivity. Also can you please write the automated test case for this fix so that we can avoid this type of errors in future. If you need any kind of help in testing let me know.

@MohitMaliFtechiz please correct me if i am wrong , Isn't oncreate always called when you open a static shortcut ? so is it necessary to handle it in onNewIntent ?

kiwix_shortcut.mp4

@MohitMaliFtechiz
Copy link
Collaborator

@shashank-0-0 No, onCreate will not always call when the application is already in the background. It will only be called when the application is fully closed(not in the background). When it is in the background and we open a static shortcut,t the onNewIntent method will call.

media_20241104_155720_4821255378590183623.mp4

@shashank-0-0
Copy link
Contributor Author

@shashank-0-0 No, onCreate will not always call when the application is already in the background. It will only be called when the application is fully closed(not in the background). When it is in the background and we open a static shortcut,t the onNewIntent method will call.

media_20241104_155720_4821255378590183623.mp4

my bad , Got it pushing the change.

Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Can you please write the automated test case for this? I will be available for any help if you need in this.

@shashank-0-0
Copy link
Contributor Author

Thanks for the changes. Can you please write the automated test case for this? I will be available for any help if you need in this.

okay!!

@shashank-0-0
Copy link
Contributor Author

@MohitMaliFtechiz I have written instrumented test for getContent shortcut can u review it, and let me know if changes are needed .

@MohitMaliFtechiz
Copy link
Collaborator

@shashank-0-0 Thanks for creating the instrumentation test cases, the code and test are good, can you please improve the commit history? there are some unrelevent commits.

Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

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

@shashank-0-0 Thanks, LGTM

@kelson42
Copy link
Collaborator

CI does not pass

@MohitMaliFtechiz
Copy link
Collaborator

@shashank-0-0 Can you please rebase? @kelson42 The CI is failing due to #4073 which we already fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GET_CONTENT shortcut breaks the BottomBar
3 participants