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

Refine the vertical gap for Section component #2747

Merged
merged 4 commits into from
Jan 3, 2025

Conversation

eason9487
Copy link
Member

Changes proposed in this Pull Request:

This is one of the preliminaries for the shipping Improvements.

Considering the way the Section component is used, it can be summarized:

  • When multiple subcomponents are presented in its body, the layout of subcomponents is always vertically oriented and requires gaps.
  • Currently, this kind of layout is achieved by wrapping VerticalGapLayout or by additional CSS.

Therefore, it should be possible to lift the vertical layout and gap to the Section component instead of the above implementation.

To avoid too many file changes for subsequent PRs, this PR refines the vertical gap for Section component:

  • Add the vertical gap prop to the Section component.
  • Remove VerticalGapLayout layers that are no longer needed.
  • Remove the vertical gap styling of the section body that is no longer needed.
  • Remove the no-longer-used 'xlarge' value from the size prop of VerticalGapLayout component.

💡 I will likely skip code review since this PR only includes developer-facing change.

Detailed test instructions:

Compare before and after this change to check if the following pages are visually consistent in their layout:

  • Onboarding flow
  • Ads onboarding flow
  • Edit free listings
  • Add/edit campaign
  • Settings, and its sub pages
    • Edit store address
    • Reconnect WordPress account flow
    • Reconnect Google account flow

Changelog entry

@eason9487 eason9487 requested a review from a team January 2, 2025 06:29
@eason9487 eason9487 self-assigned this Jan 2, 2025
@github-actions github-actions bot added the changelog: dev Developer-facing only change. label Jan 2, 2025
Copy link

codecov bot commented Jan 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.5%. Comparing base (ce6894f) to head (6462683).
Report is 5 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             develop   #2747      +/-   ##
============================================
- Coverage       67.0%   63.5%    -3.5%     
============================================
  Files            479     337     -142     
  Lines          19601    5196   -14405     
  Branches           0    1274    +1274     
============================================
- Hits           13141    3300    -9841     
+ Misses          6460    1723    -4737     
- Partials           0     173     +173     
Flag Coverage Δ
js-unit-tests 63.5% <100.0%> (?)
php-unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...onents/paid-ads/asset-group/asset-group-section.js 100.0% <100.0%> (ø)
js/src/components/paid-ads/budget-section/index.js 15.4% <ø> (ø)
js/src/components/section/index.js 100.0% <100.0%> (ø)
...nts/shipping-rate-section/shipping-rate-section.js 100.0% <100.0%> (ø)
...rc/components/stepper/step-content-footer/index.js 50.0% <ø> (ø)
js/src/components/vertical-gap-layout/index.js 100.0% <ø> (ø)

... and 810 files with indirect coverage changes

@eason9487 eason9487 merged commit 77a1f30 into develop Jan 3, 2025
9 checks passed
@eason9487 eason9487 deleted the dev/refine-section-vertical-gap branch January 3, 2025 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: dev Developer-facing only change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant