Skip to content

Conversation

@samiuelson
Copy link
Contributor

@samiuelson samiuelson commented Nov 26, 2025

WOOMOB-1801

Description

This PR adds an "Issue Refund" button to the WooCommerce POS orders screen with a placeholder dialog implementation for the refund flow.

💡 The WooPosIssueRefundDialog layout is temporary. This PR aims to just open a dialog; its layout will be implemented in next PR.

💡 Feel free to remove do not merge label once the parent branch becomes trunk.

Test Steps

  1. Verify that the Issue refund button is not showing up when the POS_REFUNDS is false.
  2. Verify that Email Receipt button works
  3. Verify that Issue Refund button opens a dialog with a temporary layout.

Images/gif

Screenshot_20251126_184347 Screenshot_20251126_184354
  • 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.

- Update `WooPosOrdersState` to handle dialog state for issuing refunds.
- Implement `WooPosIssueRefundDialog` as a placeholder for the refund flow.
- Update `WooPosOrdersDetails` to replace the "Email receipt" button with an "Issue refund" button.
- Move the "Email receipt" action into a new overflow menu in the order details view.
- Connect the refund button and dialog logic in `WooPosOrdersScreen` and `WooPosOrdersViewModel`.
@samiuelson samiuelson requested a review from Copilot November 26, 2025 16:40
Copilot finished reviewing on behalf of samiuelson November 26, 2025 16:42
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 an "Issue Refund" button to the WooCommerce POS orders screen with a placeholder dialog implementation for the refund flow. The changes also include UI refinements to the orders loading screen.

Key Changes:

  • Added "Issue Refund" button to order details with dialog state management in the ViewModel
  • Introduced WooPosIssueRefundDialog component showing a placeholder "coming soon" message
  • Moved email receipt functionality to an overflow menu, replaced by the refund button as primary action
  • Refined loading screen spacing and padding to align with the updated order details toolbar

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
WooPosOrdersViewModel.kt Adds dialog state management functions for opening and dismissing the issue refund dialog
WooPosOrdersState.kt Introduces DialogState sealed class to track whether the issue refund dialog is visible
WooPosOrdersScreen.kt Wires up the new refund button handlers and renders the dialog based on state; renames loading function
WooPosOrdersLoadingScreen.kt Updates padding and spacing to match the new toolbar layout with status bar insets
WooPosOrdersDetails.kt Replaces email receipt button with issue refund button and adds overflow menu for email receipt
WooPosIssueRefundDialog.kt New dialog component displaying placeholder UI for the upcoming refund functionality
Comments suppressed due to low confidence (1)

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersLoadingScreen.kt:169

  • Adding padding(vertical = WooPosSpacing.Small.value) after clip() applies padding outside the clipped region, which won't affect the shimmer box's appearance. If the intent is to add internal padding, it should be placed before the clip() modifier. If external spacing is needed, consider using Spacer or margin-like approaches instead.

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

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 26, 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
Commit2001de8
Direct Downloadwoocommerce-wear-prototype-build-pr15024-2001de8.apk

@samiuelson samiuelson changed the base branch from trunk to pos-orders-fix-status-bar-handling November 26, 2025 17:06
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 26, 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
Commit2001de8
Direct Downloadwoocommerce-prototype-build-pr15024-2001de8.apk

@codecov-commenter
Copy link

codecov-commenter commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 13.04348% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.58%. Comparing base (11b9108) to head (b832c4d).

Files with missing lines Patch % Lines
...ndroid/ui/woopos/orders/WooPosIssueRefundDialog.kt 0.00% 45 Missing ⚠️
...ce/android/ui/woopos/orders/WooPosOrdersDetails.kt 0.00% 35 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                            @@
##             pos-orders-fix-status-bar-handling   #15024      +/-   ##
========================================================================
- Coverage                                 38.59%   38.58%   -0.02%     
- Complexity                                10293    10299       +6     
========================================================================
  Files                                      2160     2161       +1     
  Lines                                    122503   122591      +88     
  Branches                                  16887    16900      +13     
========================================================================
+ Hits                                      47285    47297      +12     
- Misses                                    70419    70495      +76     
  Partials                                   4799     4799              

☔ 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.

@samiuelson samiuelson added this to the 23.8 milestone Nov 26, 2025
@samiuelson samiuelson added Android type: task An internally driven task. feature: POS and removed Android labels Nov 26, 2025
@samiuelson samiuelson requested a review from kidinov November 26, 2025 17:47
@samiuelson samiuelson marked this pull request as ready for review November 26, 2025 17:47
@kidinov kidinov self-assigned this Nov 27, 2025
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.

👋 @samiuelson

  • The button is visible even when FF is off - so not finished functionality will be released?
  • I am not sure about the design of the button—in Figma there is no more overflow menu without an email button at all 🤔
Image

- Propagate the `POS_REFUNDS` feature flag status through `WooPosOrdersState` and `WooPosOrdersViewModel`.
- Update `WooPosOrdersDetails` to conditionally render the "Issue Refund" button and overflow menu.
- Show a direct "Email Receipt" button when refunds are disabled.
@samiuelson
Copy link
Contributor Author

Thanks for the review, @kidinov!

  • The button is visible even when FF is off - so not finished functionality will be released?

I was convinced I've committed that 🤦 Missing commit pushed: f41a4ac

  • I am not sure about the design of the button—in Figma there is no more overflow menu without an email button at all 🤔

This UX pattern proposal comes from a designer:

  • Discussion: p1764073551626429/1763399395.564619-slack-C070SJRA8DP.
  • Docs: pdfdoF-7Av-p2

@samiuelson samiuelson added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 27, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Nov 27, 2025

1 Error
🚫 This PR is tagged with status: do not merge label(s).
1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

text = stringResource(R.string.woopos_orders_email_receipt),
onClick = { onEmailReceiptButtonClicked(details.id) },
)
if (isRefundsEnabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's not a good state model. It looks like we need to expose a list of buttons to the view, where the first one will be visible while the rest will be hidden in the overflow menu. Currently we kinda hardcode the priority in the view, while it's highly likely it will depend on the state of the specific order

Copy link
Contributor

Choose a reason for hiding this comment

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

With the list of buttons, we can pass events that each specific button emits when clicked, just like we do on the home page. Eventually, we’ll have numerous actions that can be performed on the orders.

@kidinov kidinov self-requested a review November 27, 2025 14:19
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.

I left a couple of comments but they can be implemented as a seprate PR

Also:
Image

The floating menu button doesn't match the design

- Replace `isRefundsEnabled` boolean in `WooPosOrdersState` with a list of `OrderAction` objects.
- Introduce `OrderAction` sealed interface with `IssueRefund` and `EmailReceipt` types.
- Update `WooPosOrdersViewModel` to determine available actions based on feature flags.
- Adjust `WooPosOrdersDetails` UI to render the first action as a button and remaining actions in the overflow menu.
@wpmobilebot wpmobilebot modified the milestones: 23.8, 23.9 Nov 28, 2025
@wpmobilebot
Copy link
Collaborator

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

details: OrderDetailsViewState.Computed.Details,
onEmailReceiptButtonClicked: (Long) -> Unit
actions: List<OrderAction>,
onEmailReceiptButtonClicked: (Long) -> Unit,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it's the final state or still WIP, but this is not something that is driven by the VM as you have to pass hardcoded click listeners here and everywhere instead of calling one onUIEvent that accepts events that are part of the “actions”. Also, why is OrderAction not part of the whole state but a separate class?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS status: do not merge Dependent on another PR, ready for review but not ready for merge. type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants