Skip to content

Conversation

@samiuelson
Copy link
Contributor

@samiuelson samiuelson commented Nov 14, 2025

WOOMOB-1697

Description

This PR adds functionality to disable the "cellular data" toggle in the WooCommerce POS Local Catalog settings when the device doesn't have cellular capability (data modem).

Key Changes:

  • Introduced WooPosCellularCapabilityDetector utility class to detect device telephony capabilities
  • Updated ViewModel to check cellular capability on initialization and disable the preference when unavailable
  • Modified UI to visually disable and reduce opacity of the cellular data section when capability is absent

Test Steps

Test POS > Settings > Catalog screen on different devices:

  1. Having cellular data capability
  2. Not having cellular data capability (Wi-Fi only)

Images/gif

Enabled
Screenshot_20251114_145740
Disabled
Screenshot_20251114_142715
  • 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.

@samiuelson samiuelson requested a review from Copilot November 14, 2025 13:57
@samiuelson samiuelson added this to the 23.8 milestone Nov 14, 2025
@samiuelson samiuelson added type: task An internally driven task. feature: point of sale POS project labels Nov 14, 2025
Copilot finished reviewing on behalf of samiuelson November 14, 2025 14:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to disable the "cellular data" toggle in the WooCommerce POS Local Catalog settings when the device doesn't have cellular capability (data modem).

Key Changes:

  • Introduced WooPosCellularCapabilityDetector utility class to detect device telephony capabilities
  • Updated ViewModel to check cellular capability on initialization and disable the preference when unavailable
  • Modified UI to visually disable and reduce opacity of the cellular data section when capability is absent

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
WooPosCellularCapabilityDetector.kt New utility class that checks for FEATURE_TELEPHONY system feature to determine cellular capability
WooPosSettingsLocalCatalogViewModel.kt Added cellular capability checking in init block and forces preference to false when capability is unavailable
WooPosSettingsLocalCatalogState.kt Added hasCellularCapability boolean field to track device capability state
WooPosSettingsLocalCatalogScreen.kt Updated UI to disable toggle and apply reduced opacity when device lacks cellular capability; added preview parameter provider for testing both states
WooPosSettingsLocalCatalogViewModelTest.kt Added comprehensive tests for cellular capability detection and preference handling behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 14, 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 NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commitbe15797
Direct Downloadwoocommerce-wear-prototype-build-pr14971-be15797.apk

@samiuelson samiuelson requested a review from kidinov November 14, 2025 14:07
@samiuelson samiuelson marked this pull request as ready for review November 14, 2025 14:07
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 14, 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 NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commitbe15797
Direct Downloadwoocommerce-prototype-build-pr14971-be15797.apk

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.49%. Comparing base (5cf8fcb) to head (be15797).
⚠️ Report is 82 commits behind head on trunk.

Files with missing lines Patch % Lines
...ui/woopos/util/WooPosCellularCapabilityDetector.kt 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #14971   +/-   ##
=========================================
  Coverage     38.48%   38.49%           
- Complexity    10244    10247    +3     
=========================================
  Files          2155     2156    +1     
  Lines        122223   122243   +20     
  Branches      16822    16823    +1     
=========================================
+ Hits          47036    47055   +19     
- Misses        70407    70409    +2     
+ Partials       4780     4779    -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kidinov kidinov self-assigned this Nov 18, 2025
data class WooPosSettingsLocalCatalogState(
val catalogStatus: CatalogStatus = CatalogStatus.Loading,
val allowCellularDataUpdate: Boolean = false,
val hasCellularCapability: Boolean = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

The state is designed in the way it can be ambiguous/invalid. We should do, something like that maybe. Wdyt?

          val cellularCapability: CellularCapability
      sealed class CellularCapability {
          object None : CellularCapability()
          data class Available(val allowCellularDataUpdate: Boolean) : CellularCapability()
      }

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 idea, done: c154f17

@kidinov kidinov self-requested a review November 18, 2025 11:20
Copy link
Contributor

@kidinov kidinov left a comment

Choose a reason for hiding this comment

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

Overall, LGTM.

I left one remark regarding how the state is designed.

Another one: do we even need to show this section on the devices without mobile internet?

@samiuelson
Copy link
Contributor Author

Overall, LGTM.

I left one remark regarding how the state is designed.

Another one: do we even need to show this section on the devices without mobile internet?

Thanks for review. The section is now hidden on devices without a cellular modem: 7f36fe3

@samiuelson samiuelson requested a review from kidinov November 19, 2025 17:08
Copy link
Contributor

@kidinov kidinov 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 c225db1 into trunk Nov 20, 2025
18 checks passed
@kidinov kidinov deleted the woomob-1697-disable-cellular-data-toggle-in-local-catalog-settings-in branch November 20, 2025 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants