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.

Sign up for GitHub

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

Jump to bottom

[BUG]: Navigation drawer icon is not working after returning from topics. #5266

Closed
Vishwajith-Shettigar opened this issue Dec 11, 2023 · 3 comments · Fixed by #5276
Closed

[BUG]: Navigation drawer icon is not working after returning from topics. #5266

Vishwajith-Shettigar opened this issue Dec 11, 2023 · 3 comments · Fixed by #5276
Assignees
@theMr17
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Low Solution is clear and broken into good-first-issue-sized chunks.

Comments

@Vishwajith-Shettigar
Copy link
Collaborator

Describe the bug

Navigation drawer icon is not working after returning from topics, even though user can use swipe to open navigation drawer.

Steps To Reproduce

  1. Go to home
  2. Open any topic
  3. Return to home
  4. Click on navigation drawer icon.

Expected Behavior

Clicking drawer icon should open navigation drawer.

Screenshots/Videos

navdrawerissue.webm

What device/emulator are you using?

No response

Which Android version is your device/emulator running?

No response

Which version of the Oppia Android app are you using?

No response

Additional Context

No response
@Vishwajith-Shettigar Vishwajith-Shettigar added bug End user-perceivable behaviors which are not desirable. triage needed labels Dec 11, 2023
@Vishwajith-Shettigar Vishwajith-Shettigar closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2023
@Vishwajith-Shettigar Vishwajith-Shettigar closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2023
@Vishwajith-Shettigar
Copy link
Collaborator Author

Hi @MohitGupta121, can you please tell me if this issue is able to be reproduced or not?

@theMr17
Copy link
Collaborator

theMr17 commented Dec 19, 2023

Hi @Vishwajith-Shettigar, I was able to reproduce the issue. This is probably caused by PR #5191. I am working on it.

@theMr17 theMr17 reopened this Dec 19, 2023
@theMr17 theMr17 self-assigned this Dec 19, 2023
@Vishwajith-Shettigar
Copy link
Collaborator Author

@theMr17 Thank you for confirming.

@Vishwajith-Shettigar Vishwajith-Shettigar added good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Low Solution is clear and broken into good-first-issue-sized chunks. and removed triage needed labels Dec 19, 2023
@theMr17 theMr17 mentioned this issue Dec 19, 2023
Merged
6 tasks
adhiamboperes pushed a commit that referenced this issue Dec 19, 2023
@theMr17
Fix #5266: Call setUpDrawer( ) directly if binding already initialized (
e81ba71 
#5276)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fixes #5266

This is happening because `setUpDrawer()` is not called when we go back
to a particular fragment/activity. `setUpDrawer()` is now called from
`handleCreateView()` which is called only when the fragment/activity is
created. This is fixed by calling `setUpDrawer()` directly when calling
via `onRestart()`.

|Before|After|
|--|--|
|<video
src="https://github.com/oppia/oppia-android/assets/84731134/6cf0743d-e615-4950-a3fa-4a31c147099a"
/>|<video
src="https://github.com/oppia/oppia-android/assets/84731134/42c4a7c8-47dd-44b2-817d-2f2ee4069589"
/>|



## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
BenHenning added a commit that referenced this issue Dec 23, 2023
@BenHenning
Revert "Fix #5266: Call setUpDrawer( ) directly if binding already in…
e8abe69 
…itialized (#5276)"

This reverts commit e81ba71.
@BenHenning BenHenning mentioned this issue Dec 23, 2023
Merged
BenHenning added a commit that referenced this issue Dec 23, 2023
@BenHenning
Fix part of #5284: Revert "Fix #5266: Call setUpDrawer( ) directly if…
31dfd2a 
… binding already initialized" (#5285)

Fixes part of #5284

Reverts #5276 as part of preparing to revert #5191.

Note that this reintroduces #5266 but that should stay fixed once #5191
is reverted.
BenHenning added a commit that referenced this issue Dec 23, 2023
@BenHenning
Fix part of #5284: Revert "Fix #5266: Call setUpDrawer( ) directly if…
1f465cf 
… binding already initialized" (#5285)

Fixes part of #5284

Reverts #5276 as part of preparing to revert #5191.

Note that this reintroduces #5266 but that should stay fixed once #5191
is reverted.
@tobioyelekan tobioyelekan mentioned this issue Nov 25, 2024
Merged
6 tasks
adhiamboperes pushed a commit that referenced this issue Dec 5, 2024
@tobioyelekan
Fix #5168: appcompat custom view fragment tag usage lint error (#5582)
42b1c8f 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes [#5168](#5186)
1. For `AppCompatCustomView`, replaced it with AppCompat Views
2. For `FragmentTagUsage`, replaced `<fragment/>` with
<androidx.fragment.app.FragmentContainerView/>. Made some changes to
`NavigationDrawerFragment` to:
- ensured `setUpDrawer()` is called first to initialize the
`drawerLayout`, `toolbar` and `menuItemId`
- followed by `onCreateView()` ensuring to bind the fragment
- followed on `onViewCreated()` to setup drawer functions/listeners
3. Faced an issue similar to [drawer not
working](#5266), by
minimising and resuming the app, (the drawer button stopped working).
- Fixed this by removing `onRestart()` from respective Activies making a
call to `setUpNavigationDrawer()` which at that point is not necessary,
since the fragment has been inflated initially, and would be re-inflated
if the fragment is recreated.
4. Also tested this
[bug](#5284) following the
steps to reproduce. No issues encountered

**Lint report (Before):**
![Screenshot 2024-11-24 at 6 11
50 AM](https://github.com/user-attachments/assets/479da7e9-ea3d-47d5-9c69-ff5c97992750)

**Lint report (After):**
![Screenshot 2024-11-24 at 8 06
27 AM](https://github.com/user-attachments/assets/68ef6cb9-b9b6-45ac-a901-ed64540ee674)

## Screen Record


https://github.com/user-attachments/assets/6db98657-d470-4263-a5ac-d055f4d64e66



## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@theMr17 theMr17

Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Low Solution is clear and broken into good-first-issue-sized chunks.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #5266: Call setUpDrawer( ) directly if binding already initialized theMr17/oppia-android
2 participants
@Vishwajith-Shettigar @theMr17