-
Notifications
You must be signed in to change notification settings - Fork 78
feat(FR-1662): use checkbox for openToPublic option in model service form
#4606
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
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🔴 | Statements | 4.67% (-0.02% 🔻) |
531/11377 |
| 🔴 | Branches | 3.77% (-0.05% 🔻) |
302/8016 |
| 🔴 | Functions | 2.89% (-0.01% 🔻) |
102/3526 |
| 🔴 | Lines | 4.61% (-0.03% 🔻) |
513/11120 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / ImportNotebook.tsx |
0% | 100% | 0% | 0% |
Show files with reduced coverage 🔻
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🟢 | ... / graphql-transformer.ts |
82.26% (-4.92% 🔻) |
79.41% (-5.2% 🔻) |
71.43% (-3.57% 🔻) |
83.61% (-4.71% 🔻) |
| 🔴 | helper/index.tsx | 42.93% (-2.23% 🔻) |
40.12% (-0.73% 🔻) |
26.53% (-1.13% 🔻) |
43.5% (-2.01% 🔻) |
Test suite run success
121 tests passing in 14 suites.
Report generated by 🧪jest coverage report action from 9ee45d1
Co-authored-by: Seungwon Lee <seungwon@lablup.com>
There was a problem hiding this 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 replaces the Switch component with a Checkbox component for the "Open to Public" option in the model service launcher form, improving the UI/UX for this form field.
Key Changes
- Replaced
SwitchwithCheckboxcomponent in the service launcher form - Updated imports to include
Checkboxand removeSwitchfrom antd imports - Moved the label text from the Form.Item
labelprop to become children of the Checkbox component
nowgnuesLee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you apply the copilot review? i think it makes sense.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
nowgnuesLee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
yomybaby
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
resolves #4605 (FR-1662)
This pull request updates the UI component for the "Open to Public" option on the
ServiceLauncherPageContentform. The main change is replacing theSwitchcomponent with aCheckbox, and updating the logic to handle its state within the form.UI Component Changes:
Switchcomponent with aCheckboxfor the "Open to Public" field, updating the import and usage inServiceLauncherPageContent.tsx. [1] [2]getFieldValueandsetFieldValue.Checklist: (if applicable)