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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UI] Make the login screen and bottom navigation view more tablet friendly. #20957

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from

Conversation

notandyvee
Copy link
Contributor

@notandyvee notandyvee commented Jun 7, 2024

This PR attempts to make the bottom navbar "adaptive" so when on larger screens it puts the navbar on the left side instead. While using my foldable I noticed we could be utilizing the space better.

Before After
Screenshot_20240607_152510 Screenshot_20240607_152615

Unfortunately we are not using Jetpack Compose with the bottom navbar, so properly doing adaptive navigation is tricky. Especially since it's a custom view. So I utilized two different techniques to make it work.

First, I used the xml file qualifiers that automatically choose layouts for certain screen sizes. This allowed me to move the navigation view to the right spots.

WordPress/src/main/res/layout-sw600dp/main_activity.xml

Second, the BottomNavigationView and the NavigationRailView extend from the same parent. Meaning I was able to just reference the parent class. All the related components have similar inheritance structures.

The downside is main_activity.xml is duplicated in order to make the adaptive part work. But overall surprised it wasn't worse.


Additionally on the login screen I noticed weird spacing on the login button. So I fixed that.

Before After
Screenshot_20240607_152409 Screenshot_20240607_152641

Lastly, while testing an actual tablet emulator I noticed that the login screen is actually centered and does not utilize the full screen space in landscape. The experience is pretty poor. So I fixed that too. Sorta.

Month Savings
Screenshot_20240607_133646 Screenshot_20240607_140615

To Test:

Login

  • Test using a phone. Ensure the layout of the Login screen works.
  • Using a foldable ensure folded and unfolded the layout looks fine.
  • Using a tablet ensure the login screen layout looks fine.

Adaptive Navigation

  • 馃毈 Smoke test a regular phone device. The navigation should work exactly as it did before.
  • Using a physical foldable or foldable emulator, test that the apps navigation changes based on whether you are folded vs no folded.
  • Using a physical tablet or emulator, ensure the navigation is adaptive. Depending on the screen size portrait could be the navigation rail or bottom nav. I would expect landscape to definitely be the navigation rail on the left side.

Smoke Testing

  • 馃毈 Test the app and visit as many screens as you can think of. Make sure the theme looks fine.

Known Issues

Please note I realize that the tablet login screen looks off due to the spacing of the vertical scrolling letters. That screen probably needs additional love. Just want to gather some feedback.


Regression Notes

  1. Potential unintended areas of impact

    The existing bottom navbar..

  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    Manual testing.

  3. What automated tests I added (or what prevented me from doing so)

    N/A


PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Additionally also fixed an issue where the app would not poperly
layout in tablet landscape mode. This occurred due to the
portrait limitation on the login activity.
@dangermattic
Copy link
Collaborator

dangermattic commented Jun 7, 2024

1 Warning
鈿狅笍 PR is not assigned to a milestone.

Generated by 馃毇 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 7, 2024

Jetpack馃摬 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20957-d1ed133
Commitd1ed133
Direct Downloadjetpack-prototype-build-pr20957-d1ed133.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 7, 2024

WordPress馃摬 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20957-d1ed133
Commitd1ed133
Direct Downloadwordpress-prototype-build-pr20957-d1ed133.apk
Note: Google Login is not supported on these builds.

@notandyvee
Copy link
Contributor Author

Something to note. I added padding to the navigation rail. It's 0dp for the bottom nav. So no change. This is per the material spec:

Screenshot 2024-06-14 at 4 22 15鈥疨M

Unfortunately there is no guidance around how to handle that. We could deviate from the spec to increase the width. But if a users font is larger, it wouldn't matter too much. I think for now it's fine. But something to think about.


I also improved the logo spacing in landscape.

Screenshot 2024-06-13 at 4 35 07鈥疨M

We can further improve this by laying out the UI slightly different in landscape. But didn't want to make too many edits.

@notandyvee notandyvee marked this pull request as ready for review June 14, 2024 21:01
@notandyvee
Copy link
Contributor Author

Just a heads up. Had to make a larger theme change due to a lint issue for potential overdraw. For some reason the BottomNavigationView wasn't being properly styled. After investigating the reason was using Theme.MaterialComponents.Light.DarkActionBar.Bridge. I took a look at the material components and see no mention of "bridge". So instead used Theme.MaterialComponents.DayNight. I changed it for the night version too. Just want to clarify the reasoning. I ran tests on a nexus 5, pixel tablet, and my Z Fold 5. Theming appears to be intact. But if you notice something funky, that is likely the culprit.

Copy link

sonarcloud bot commented Jun 17, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

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.

None yet

3 participants