-
Notifications
You must be signed in to change notification settings - Fork 149
Added field name automatic slugification. #6
Conversation
.toLowerCase() | ||
.trim() | ||
.replace(/\s/g, "_") | ||
.replace(/[^a-z0-9]/, ""); |
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.
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'm gonna use https://www.npmjs.com/package/slug
62f1a04
to
d00a42a
Compare
5671d4c
to
4b2ea8f
Compare
1858757
to
8fc76a6
Compare
@@ -24,6 +25,10 @@ function clone(obj) { | |||
return JSON.parse(JSON.stringify(obj)); | |||
} | |||
|
|||
function unique(array) { |
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.
Should we rename this to deduplicateArray
?
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.
Huh. I thought unique
was meaningful enough :)
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.
unique is not a verb, that's why I was mistaken.
If I'm not mistaken, the user sees the name of the field and its id, and can update both of them. What about having only the name exposed, and having the id hidden from the user? I believe it's not something the users need to know. What do you think? Otherwise, the code changes looks good to me. |
Well if you plan on building the right JSON schema for third-party tools requiring proper field naming? That's really important imho (think kinto-admin or any tool wanting to retrieve the json schema definition). I understand that for totally non-technical users just wanting a shareable UI it might be unwanted/get in the way, but I think we shouldn't drive the implementation by their sole needs... Maybe we could iterate on this, for example by allowing a configureable option to hide the name field (which is what you call the id); though that should probably be covered by a new issue. |
that works for me :) |
I'm landing this. Filed #8 for hiding the name field. |
Added field name automatic slugification.
This patch adds support for automatic field name generation when a new title is entered for the first time. It also adds unit and functional tests for the
form
reducer and theEditableField
component.r=? @almet