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

[Android SDK] Edge to edge main activity doesnt handle well 3 button navigation #13460

Conversation

kidinov
Copy link
Contributor

@kidinov kidinov commented Feb 4, 2025

Closes: #13455

Description

I noticed that as not all fragments in the main activity use bottom navigation, then in the cases when 3 buttons navigation is used, part of the fragment is hidden behind the navigation. The ideal solution would be to adopt all the fragments, but it's out of the scope for the target SDK project

Also, thanks to @atorresveiga, I realized that the main activity doesn't handle the case when a phone in landscape mode with three-button navigation, then the buttons are placed on a side (I beleave that in older versions they were on the bottom)
This PR also addresses that

Testing information

  • Try Android 35 and below
  • Try landscape and portrait mode with 3 button navigation enabled and without
  • Try different fragments on the main screen

The tests that have been performed

above

Images/gif

image image
  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

…t itself. Ideal solution would be adding paddings where needed in the fragments but it's 23 fragments in there so a bit out of scope for updating target sdk project and should be a separate one
@kidinov kidinov added this to the 21.7 milestone Feb 4, 2025
@kidinov kidinov requested a review from samiuelson February 4, 2025 14:06
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 4, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit43d92e5
Direct Downloadwoocommerce-wear-prototype-build-pr13460-43d92e5.apk

@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit43d92e5
Direct Downloadwoocommerce-prototype-build-pr13460-43d92e5.apk

@wpmobilebot wpmobilebot modified the milestones: 21.7, 21.8 Feb 7, 2025
@wpmobilebot
Copy link
Collaborator

Version 21.7 has now entered code-freeze, so the milestone of this PR has been updated to 21.8.

@samiuelson samiuelson self-assigned this Feb 10, 2025
Copy link
Contributor

@samiuelson samiuelson left a comment

Choose a reason for hiding this comment

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

LGTM!

@kidinov kidinov merged commit c052c7b into 13270-android-sdk-update-target-sdk-to-35 Feb 11, 2025
15 of 20 checks passed
@kidinov kidinov deleted the 13455-android-sdk-edge-to-edge-main-activity-doesnt-handle-well-3-button-navigation branch February 11, 2025 13:26
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.

3 participants