-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Generate a two-word project name #184
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
Conversation
@@ -1,7 +1,10 @@ | |||
import * as ActionTypes from '../../../constants'; | |||
const generate = require('project-name-generator'); |
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.
To ES6-ify this line:
import generate from 'project-name-generator';
|
||
const initialState = { | ||
name: 'Hello p5.js' | ||
name: `${generatedName}` |
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.
You should move the function call into the initial state, i.e. change this to
const initialState = {
name: `${generate({ words: 2}).dashed}`
};
As it is now, the following happens, which I think is a bug:
- User navigates to the editor and starts working on a project
- User saves the project.
- User clicks "New"
- The new project has the same name as the previous project.
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.
i tried this and found that moving the function call doesn't fix the problem, i think because newProject() doesn't actually create a new project; it just returns the initialState name. this was fine when all new sketches had the same name. hmm, i'll have to think about this a little more... has there been discussion of "untitled" as the sketch name prior to saving? and then the generated name appears on save?
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.
Oh, right! The way to make this work is to make initialState into a function rather than a variable. See the files reducer for an example of how to do this.
I'm not saying that I don't like it, but is there any reason you decided that the name should be dashed rather than spaced? I'm not sure that I have an opinion about it, just curious. Also, do you think that the first work should be capitalized? Am I thinking about this too hard? |
I'm leaning towards first word capitalized + spaced rather than dashed, I think. |
061c7c6
to
81475c0
Compare
Closes #101
