Skip to content
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

Update new user registration questions #2054

Merged
merged 2 commits into from
Oct 22, 2024
Merged

Conversation

kabilar
Copy link
Member

@kabilar kabilar commented Oct 21, 2024

Supersedes #2038

@waxlamp
Copy link
Member

waxlamp commented Oct 22, 2024

@kabilar, @mvandenburgh: I moved the parenthesis from the question itself down into a bolded, italicized div right above the submit button. Take a look and lmk what you think. Particularly:

  • @kabilar: is it ok to have the admonition there instead of in the question?
  • @mvandenburgh: there is still trouble with the styling for those form fields (crunch your screen width down to see what I mean) but at least it's less likely with the shorter question -- is that ok for now at least?
    • Related question: should we bother to "fix" this? It doesn't seem worth the effort. Thoughts?

Note also that the dandi-cli integratin test is failing due to the fork not having access to the repo secrets. In this case I don't think it matters because this is purely a cosmetic change in the user signup flow (which doesn't affect the CLI). But in the future, there are apparently ways to share the appropriate secrets with forks and we can look into that.

Copy link
Member

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

This looks good to me (pending @kabilar and @mvandenburgh verbally approving my change).

This helps avoid the "crowding" of text that occurs in the form field
itself.
@mvandenburgh
Copy link
Member

Looks good to me 👍
Screenshot from 2024-10-22 12-40-45

@kabilar
Copy link
Member Author

kabilar commented Oct 22, 2024

Looks good to me 👍 Screenshot from 2024-10-22 12-40-45

Hi @mvandenburgh, this looks like the current form.

@kabilar
Copy link
Member Author

kabilar commented Oct 22, 2024

  • @kabilar: is it ok to have the admonition there instead of in the question?

Hi @waxlamp Yes, thank you. I just noticed the note at the top of the form. Perhaps we should remove this note or replace it with the phrasing that I suggested.

@mvandenburgh
Copy link
Member

Sorry for the confusion, here's the correct form
Screenshot from 2024-10-22 13-28-33

@waxlamp
Copy link
Member

waxlamp commented Oct 22, 2024

  • @kabilar: is it ok to have the admonition there instead of in the question?

Hi @waxlamp Yes, thank you. I just noticed the note at the top of the form. Perhaps we should remove this note or replace it with the phrasing that I suggested.

I think the repetition is probably warranted here. I'm going to go ahead and merge, and if we want to de-duplicate the warnings, we can always do so later.

@waxlamp waxlamp added patch Increment the patch version when merged release Create a release when this pr is merged labels Oct 22, 2024
@waxlamp waxlamp merged commit bccbc2d into dandi:master Oct 22, 2024
9 of 10 checks passed
@dandibot
Copy link
Member

🚀 PR was released in v0.3.107 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants