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

Add name to site creation form, but autopopulate based on title input #134

Closed
wants to merge 1 commit into from

Conversation

mbertrand
Copy link
Member

@mbertrand mbertrand commented Mar 22, 2021

Blocked until a final decision is made on whether or not to allow non-ascii urls, not a high priority yet.

Pre-Flight checklist

  • Screenshots and design review for any changes that affect layout or styling
    • Desktop screenshots
    • Mobile width screenshots
    • Tag @Ferdi or @pdpinch for review
  • Testing
    • Code is tested
    • Changes have been manually tested

What are the relevant tickets?

Closes #128

What's this PR do?

Adds a name input field to the site creation form, autopopulates it with a slug based on the title field input, and prevents non-ascii characters from being used.

How should this be manually tested?

Go to the form for creating a new site. As you enter a title, the name field below should start to populate. It should contain only lowercase a-z, 0-9, and - characters. If you try to add other characters you should see a validation error. Submit the form, the site should be created with that name/slug.

Where should the reviewer start?

Initially thought about allowing non-ascii characters in the name by passing allow_unicode=True to django.utils.text.slugify, but after talking with Ferdi & Nathan decided to disallow it and use this approach instead.

Screenshots

Screen Shot 2021-06-14 at 3 22 36 PM

@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2021

Codecov Report

Merging #134 (9553f09) into master (e32d103) will decrease coverage by 0.11%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
- Coverage   88.31%   88.19%   -0.12%     
==========================================
  Files         123      123              
  Lines        3638     3644       +6     
  Branches      549      548       -1     
==========================================
+ Hits         3213     3214       +1     
- Misses        387      392       +5     
  Partials       38       38              
Impacted Files Coverage Δ
static/js/pages/SiteCreationPage.tsx 86.48% <ø> (-0.36%) ⬇️
static/js/types/websites.ts 100.00% <ø> (ø)
static/js/components/forms/SiteForm.tsx 78.26% <28.57%> (-21.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e32d103...9553f09. Read the comment docs.

@mbertrand mbertrand added Blocked and removed Blocked labels Jun 14, 2021
@mbertrand
Copy link
Member Author

This is really out of date so I'm going to close it until the related issue becomes a priority.

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

Successfully merging this pull request may close these issues.

misleading error message when trying to create a site with non-latin characters
2 participants