-
Notifications
You must be signed in to change notification settings - Fork 902
feat(FormField): add required
field support across form components
#4687
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
base: v4
Are you sure you want to change the base?
Conversation
required
field support across form components
sorry i make pull request again because i`m accidently remove my repository @benjamincanac |
commit: |
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
) Co-authored-by: Sandro Circi <[email protected]>
Co-authored-by: Benjamin Canac <[email protected]>
a3fc9ca
to
13eecb1
Compare
@rdjanuar Would you be willing to move this PR to v4? |
return { | ||
[props.orientation === 'vertical' ? 'height' : 'width']: percent.value ? `${percent.value}%` : 'fit-content' | ||
} | ||
const value = `${Math.max(percent.value ?? 0, 0)}%` |
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.
Hi! I was looking at this PR as it seems to have gotten stuck, and I wanted to gently suggest a couple of things that might help the maintainers handle it more easily.
-
Unrelated Changes ("Code Churn"): This change, for example, doesn't seem to be related to the PR's main goal. I've also noticed other similar changes throughout the files, like updates to CSS. To make the core change clearer, it would be helpful if these unrelated changes could be moved to a separate PR (like "Simplify code").
-
There are currently 33 commits in this PR. For a focused change like this, it would be better if they could be rebased and squashed down into a single commit.
No offense β I was actually going to come up with a similar request, and now I see there's even a ready made PR, and to be honest I understand why it is.
@benjamincanac I struggle with package json EOL and pnpm.lock, would you mind to help me ? |
π Linked issue
Resolves #4529
β Type of change
π Description
π Checklist