Skip to content

Conversation

@RafaelKayumov
Copy link
Contributor

@RafaelKayumov RafaelKayumov commented May 16, 2025

Closes: WOOMOB-476

Description

The change in this PR fixes overlapping and shifting layout after applying a larger dynamic-type size.

  • Deletes occurrences .frame(idealHeight: Layout.rowHeight) from WooShippingCreateLabelsView
  • Replaces AdaptiveStack with HStack. Adaptive stack flips into a vertical stack when there is not enough width (widths becomes compact) making inline components to re-arrange vertically even if there is still some space available.
  • Restricts the "notify the customer" toggle label lines by 3 to avoid the CTA area of bottom sheet growing too much and covering the expandable content.
  • Restricts the toggle label maximum dynamic type size for same reason.
  • Updates the top tabs selection indicator in case of dynamic type size change.

Steps to reproduce

Testing information

  • Log in to a test store with Woo Shipping plugin set up.
  • Navigate to the Orders tab and select a completed order with at lest one shipment with a purchased label and at least one shipment without a label.
  • Navigate to "Create shipping labels"
  • Use option+command + (+/-) combination to adjust dynamic type size or do it manually in Settings -> Accessibility -> Display & Text Size -> Larger Text.
  • Follow steps in demo video examples. Content should be readable and should not overlap.

Before - Album

Horizontal_Broken.mp4

After - Album

Horizontal_Fixed.mp4

Before - Portrait

Simulator.Screen.Recording.-.iPhone.16.-.2025-05-16.at.14.00.21.mp4

After - Portrait

Simulator.Screen.Recording.-.iPhone.16.-.2025-05-16.at.13.57.25.mp4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

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 all devices (phone/tablet) and no regressions are added.

@RafaelKayumov RafaelKayumov added this to the 22.5 milestone May 16, 2025
@RafaelKayumov RafaelKayumov added the type: enhancement A request for an enhancement. label May 16, 2025
@RafaelKayumov RafaelKayumov force-pushed the WOOMOB-476-fix-shipping-labels-dynamic-layout branch from 470a467 to 9adb76c Compare May 16, 2025 10:41
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented May 16, 2025

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

App NameWooCommerce iOS Prototype
Build Number30044
VersionPR #15648
Bundle IDcom.automattic.alpha.woocommerce
Commitfe75fb8
Installation URL02qjnmr3c4kq8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@RafaelKayumov RafaelKayumov force-pushed the WOOMOB-476-fix-shipping-labels-dynamic-layout branch from 9adb76c to 12813d1 Compare May 16, 2025 11:35
@RafaelKayumov RafaelKayumov marked this pull request as ready for review May 16, 2025 11:56
@RafaelKayumov RafaelKayumov requested a review from itsmeichigo May 16, 2025 11:56
@itsmeichigo itsmeichigo self-assigned this May 19, 2025
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Tested and confirmed using simulator iPhone 16 Pro with accessibility font sizes.

.footnoteStyle()
}
AdaptiveStack {
HStack {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: setting top alignment would make the layout look nicer in this case:

image

.frame(idealHeight: Layout.rowHeight)
ForEach(viewModel.shippingLines) { shippingLine in
AdaptiveStack {
HStack {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same suggestion as above about the child item alignment.


static let toggleTextLineLimit = 3
static let toggleTextMaxDynamicTypeSizePortrait = DynamicTypeSize.accessibility1
static let toggleTextMaxDynamicTypeSizeAlbum = DynamicTypeSize.xxxLarge
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nit: can we use landscape wording instead of album? I'm unfamiliar with this wording, which may be hard to understand.

Comment on lines +167 to +175
.onChange(of: geometry.size) { newSize in
/// Support dynamic type size change
if index < tabWidths.count {
tabWidths[index] = newSize.width
if index == selectedTab {
underlineTabWith(tabGeometry: geometry)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice fix!

@RafaelKayumov RafaelKayumov merged commit 28502c8 into trunk May 23, 2025
13 checks passed
@RafaelKayumov RafaelKayumov deleted the WOOMOB-476-fix-shipping-labels-dynamic-layout branch May 23, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement A request for an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants