Skip to content

Conversation

rosalieper
Copy link
Contributor

@rosalieper rosalieper commented Sep 19, 2025

Bug: T404988

Copy link

Deployment previews on netlify for branch refs/pull/1042/merge will be at the following locations (when build is done):

Copy link
Member

@outdooracorn outdooracorn left a comment

Choose a reason for hiding this comment

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

Looking good, I have a couple of comments

</v-radio>
</v-radio-group>

<h3 v-if="showTerms" class="mt-6">Terms of Use</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Looking at Anton's mockup in the ticket, the ToU heading should remain (I guess just without the v-if="showTerms")

</v-checkbox>
<div>
Previously accepted
<v-tooltip bottom>
Copy link
Member

Choose a reason for hiding this comment

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

Nice tooltip! 👍

Copy link
Member

Choose a reason for hiding this comment

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

If we can get the max-width of the card to be bigger, I like having the tooltip above the link - there is more space there.

image

Comment on lines 57 to 71
<div>
Previously accepted
<v-tooltip bottom>
<template v-slot:activator="{ on }">
<a
target="_blank"
href="/terms-of-use"
@click.stop
v-on="on"
>
Terms of Use</a>
</template>
Opens in new window
</v-tooltip> still apply.
</div>
Copy link
Member

Choose a reason for hiding this comment

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

The text here is smaller than the rest of the form and Anton's mockup. A text-body-1 class on the div should do the trick. However, is the text intentionally smaller because it doesn't fit on a single line otherwise?

Let's ask UX if we should use a smaller font size or can increase the max-width of this card to 400px (technically 395px is the minimum it has to be for the text to fit).

Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants