Skip to content

[FIX] Side menu collapses info in landscape #4580

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

Merged
merged 3 commits into from
Apr 30, 2025

Conversation

joragua
Copy link
Collaborator

@joragua joragua commented Apr 29, 2025

Related Issues

App: #4513

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

@joragua joragua self-assigned this Apr 29, 2025
@joragua joragua linked an issue Apr 29, 2025 that may be closed by this pull request
@joragua joragua marked this pull request as ready for review April 29, 2025 16:11
@joragua joragua requested a review from jesmrec April 29, 2025 16:11
@jesmrec
Copy link
Collaborator

jesmrec commented Apr 30, 2025

LGTM 👍

@jesmrec
Copy link
Collaborator

jesmrec commented Apr 30, 2025

QA checks:

  • Drawer in portrait
  • Drawer in landscape
  • Drawer open and changing orientation
  • Talkback in the drawer
  • Keyboard moving through the drawer
  • Logo + Link in portrait
  • Logo + Link in landscape
  • Logo + Link in drawer open and changing orientation

@jesmrec
Copy link
Collaborator

jesmrec commented Apr 30, 2025

After checking the accessibility regards on this issue and going back to our history to find the original cause, i stumbled upon the following QA report:

#4454 (comment)

The problem caused in the accessibility wasn't the blank spaces themselves, but the numbering in the dictation. The fix introduced in the current PR does not fix this, since all the items in the drawer list are pronounced as "x of 7" when there are only 4 (i miss one¿?)

I tested by removing the android:importantForAccessibility field, and the result was the same. I'd keep the field there anyway.

Therefore, we need a compromise solution:

  • Keep the bad numbering dictation that allows the UI glitch to be removed
  • Fix the numbering dictation, but UI could mess again
  • (any other...?)

@jesmrec
Copy link
Collaborator

jesmrec commented Apr 30, 2025

Since there is no a totally fittable solution, we'll go for the first approach:

Keep the bad numbering dictation that allows the UI glitch to be removed

there are other risky solutions that can lead to other bigger problems and bad UI/UX situations that are not worthy at this point.

Finally:

  • We assume that numeration of items in the drawer is not totally correct and excedes the number of options. It could lead to confusion to visual impaired users, but, it does not hurt the UX.
  • Drawer's look improved in both orientations, letting users to scroll options without overlapped text.

As usual, new feedback to receive is always welcome and we are open to new improvements and iterations in the feature.

Agreed @joragua?

@joragua
Copy link
Collaborator Author

joragua commented Apr 30, 2025

Yeah, I agree. There could be another compromise solution but I think that, at the moment we are, is not feasible: modify the drawer layout and have an empty block between the menu and the footer (for example). I've tried it but the drawer collapses many times so, in my opinion, it is not worthy to work on it. 🤔 In this case, I would prefer to keep the bad numbering dictation from Talkback and fix the drawer glitch.

@jesmrec
Copy link
Collaborator

jesmrec commented Apr 30, 2025

Approved, then.

@joragua joragua merged commit fafdf8a into master Apr 30, 2025
8 checks passed
@joragua joragua deleted the fix/drawer_collapses_in_landscape branch April 30, 2025 10:35
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.

[BUG] Side menu collapses info in landscape
3 participants