Skip to content

Conversation

@chaimann
Copy link
Contributor

@chaimann chaimann commented Jun 4, 2025

Note

Split from #6160

Summary

We have a couple of places throughout the admin where checkboxes are used as part of a form, with all the same elements being reused to construct a checkbox field:

  • label with some caption;
  • hidden input field to support sending a "0" value in request;
  • optional toggletip hint;

Few places however lacked the hidden input, therefore the form was not submitting the "0" value correctly.

This updated component builds a reusable form checkbox, that encapsulates all the needed layout and logic for a form checkbox to be displayed and work properly, with an interface to customise it to each form needs.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

@codecov
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.95%. Comparing base (6d66dad) to head (c64ed78).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6275      +/-   ##
==========================================
+ Coverage   88.93%   88.95%   +0.01%     
==========================================
  Files         859      860       +1     
  Lines       18407    18423      +16     
==========================================
+ Hits        16371    16388      +17     
+ Misses       2036     2035       -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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chaimann chaimann mentioned this pull request Jun 6, 2025
4 tasks
@chaimann chaimann force-pushed the admin-ui-checkbox-refactor branch from 901894e to 12158f8 Compare June 6, 2025 17:20
@chaimann chaimann marked this pull request as ready for review June 10, 2025 14:14
@chaimann chaimann requested a review from a team as a code owner June 10, 2025 14:14
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Very nice I have a simple wording nit.

checked: @checked,
**@attributes
) %>
<%= caption %>
Copy link
Member

Choose a reason for hiding this comment

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

Caption seems like a weirdly named attribute for the label. Can we name this duck as it quacks please? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah there was a reason why I chose to call it "caption" but I don't really remember now 😄 will change it to "label"

chaimann added 4 commits June 11, 2025 12:46
This is the first step to refactor checkbox component, so it can be used
as a separate element or as a form control.
Motivation:
We have a couple of places throughout the admin where checkboxes
are used as part of a form, with all the same elements being reused
 to construct a checkbox field:
 - label with some caption;
 - hidden input field to support sending a "0" value in request;
 - optional toggletip hint;
Few places however lacked the hidden input, therefore the form was
not submitting the "0" value correctly.

This updated component builds a reusable form checkbox, that
encapsulates all the needed layout and logic for a form checkbox to
be displayed and work properly, with an interface to customise it
to each form needs.
"label" is more suitable here than "caption"
@chaimann chaimann force-pushed the admin-ui-checkbox-refactor branch from 12158f8 to c64ed78 Compare June 11, 2025 10:55
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

nice work

@tvdeyen tvdeyen enabled auto-merge June 11, 2025 11:16
@tvdeyen tvdeyen merged commit ffa13fc into solidusio:main Jun 11, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants