feat(elements,ui,clerk-js): Legal consent elements support and improvements #4427
Conversation
🦋 Changeset detectedLatest commit: d8648b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
octoper
force-pushed
the
vaggelis/user-744-implement-legal-consent-on-elements
branch
from
bcb2dc6
to
ab45205
Compare
octoper
force-pushed
the
vaggelis/user-744-implement-legal-consent-on-elements
branch
from
9ac9d17
to
e8a3406
Compare
octoper
changed the title
There was a problem hiding this comment.
Elements stuff generally looks good to me, I'll leave UI up to @alexcarpenter.
packages/elements/src/internals/machines/sign-up/utils/fields-to-params.ts
Outdated
Show resolved
Hide resolved
octoper
force-pushed
the
vaggelis/user-744-implement-legal-consent-on-elements
branch
from
2df32f8
to
5ad10db
Compare
|
|
||
| <Common.Label asChild> | ||
| <Field.Label> | ||
| <span> |
There was a problem hiding this comment.
is there a reason for the extra wrapping span here?
There was a problem hiding this comment.
Yeah because the <Field.Label/> components has a flex and I cant override id as the classes are not merged so I wrapped it in a span to escape it
There was a problem hiding this comment.
Ok, maybe we need a prop on the label to set the flex direction? Trying to reduce the amount of extra markup added to reduce the need for descriptors.
| <Card.Actions> | ||
| {legalConsentEnabled && hasIdentifier && <LegalAcceptedField />} |
There was a problem hiding this comment.
Similar to the continue step, I feel like this should live within the body not the actions.
There was a problem hiding this comment.
This is how we render it on AIO right now, it's closer to the Continue button when sign up has identifier, when not having identifier and only having social buttons we put this closer to the social button (within the body), if this feels a bit close to the continue button it's probably because we need to adjust the gap
There was a problem hiding this comment.
If this still does not look right to you, we can bring Alvish to take a look and after that I will make the appropriate changes
There was a problem hiding this comment.
ok, either way lets make it consistent. Looks like in the continue step its placed in the body.
There was a problem hiding this comment.
Yeah it's different there though as we wanted to have it on the body, because it's one of the missing fields and it didn't looked right when we were looking the designs with Alvish
| asChild | ||
| > | ||
| <Field.Root> | ||
| <div className='flex justify-center gap-2'> |
There was a problem hiding this comment.
Will want to replace the gap-2 usage here with padding left on the label.
octoper
force-pushed
the
vaggelis/user-744-implement-legal-consent-on-elements
branch
from
9fcb62f
to
0a36c31
Compare
packages/elements/src/internals/machines/sign-up/continue.machine.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
👍🏼 will tackle any tweaks in a follow up pr. Thanks @octoper!
…to-params.ts Co-authored-by: Tom Milewski <[email protected]>
octoper
force-pushed
the
vaggelis/user-744-implement-legal-consent-on-elements
branch
from
8758130
to
eadc5de
Compare
octoper
deleted the
vaggelis/user-744-implement-legal-consent-on-elements
branch
Description
This PR adds Elements support for Legal consent feature, it also includes implementation of the UI build with Elements and includes a couple improvements on the AIO components
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change