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

[Woo POS] Add public-facing documentation link to POS popup menu #13466

Merged
merged 9 commits into from
Feb 11, 2025

Conversation

AnirudhBhat
Copy link
Contributor

@AnirudhBhat AnirudhBhat commented Feb 5, 2025

Closes: #13454

NOTE: The POS documentation link currently returns a 404, but it is planned to be ready before the public release.

Description

This PR introduces a new popup menu item in the POS interface that allows users to access public-facing documentation. Currently, a placeholder URL is used as the final URL is yet to be finalised.

Discussion: p91TBi-cNJ-p2#comment-13978

Changes

  1. Added a new "View Documentation" menu item in the POS popup menu.
  2. Configured it to open a dummy URL (to be updated once the final link is available).
  3. Added unit test

Testing information

  1. Verified that the new menu item appears correctly in the POS popup menu.
  2. Confirmed that clicking it opens the dummy URL in a browser.
  3. Ensured no regressions in existing menu functionality.

The tests that have been performed

Tested both on Light and Dark mode

Images/gif

Dark Light
Screenshot 2025-01-02 at 9 30 20 AM
Screen_recording_20250205_100820.mp4
  • 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.

@AnirudhBhat AnirudhBhat added type: task An internally driven task. status: do not merge Dependent on another PR, ready for review but not ready for merge. labels Feb 5, 2025
@AnirudhBhat AnirudhBhat added this to the 21.8 milestone Feb 5, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 5, 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
Commit88ee6c8
Direct Downloadwoocommerce-wear-prototype-build-pr13466-88ee6c8.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 5, 2025

📲 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
Commit88ee6c8
Direct Downloadwoocommerce-prototype-build-pr13466-88ee6c8.apk

@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2025

Codecov Report

Attention: Patch coverage is 42.10526% with 11 lines in your changes missing coverage. Please review.

Project coverage is 37.89%. Comparing base (8418862) to head (88ee6c8).
Report is 153 commits behind head on trunk.

Files with missing lines Patch % Lines
...ce/android/ui/woopos/home/toolbar/WooPosToolbar.kt 0.00% 10 Missing ⚠️
...d/ui/woopos/home/toolbar/WooPosToolbarViewModel.kt 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #13466   +/-   ##
=========================================
  Coverage     37.89%   37.89%           
- Complexity     8957     8959    +2     
=========================================
  Files          2050     2050           
  Lines        112086   112104   +18     
  Branches      14172    14174    +2     
=========================================
+ Hits          42477    42486    +9     
- Misses        65730    65739    +9     
  Partials       3879     3879           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AnirudhBhat AnirudhBhat marked this pull request as ready for review February 7, 2025 03:27
Copy link
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @AnirudhBhat! Works as expected. I've left a couple minor comments, feel free to merge this.

@@ -239,7 +249,7 @@ private fun PopUpMenu(
onClick: (Menu.MenuItem) -> Unit
) {
WooPosCard(
modifier = modifier.width(214.dp),
modifier = modifier.width(254.dp),
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Feels a bit off to hardcode a specific value, considering we don't know what font size and language the user will be using. What do you think about setting max-width instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've made it dynamic here: 88ee6c8

WooCommerce/src/main/res/drawable/ic_woo_pos_info.xml Fixed Show resolved Hide resolved
@AnirudhBhat AnirudhBhat removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Feb 11, 2025
@AnirudhBhat
Copy link
Contributor Author

Considering this is a very simple icon, I'm wondering if we can do something about this warning. Have you tried simplifying the vector using one of the online tools?

I've optimised the icon here: 88ee6c8

@AnirudhBhat AnirudhBhat merged commit b483dbb into trunk Feb 11, 2025
16 checks passed
@AnirudhBhat AnirudhBhat deleted the issue/13454-pos-public-facing-documentation branch February 11, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task An internally driven task.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Woo POS] Add a Link to Public-Facing Documentation in POS Interface
4 participants