-
Notifications
You must be signed in to change notification settings - Fork 533
Lack of ID creates errors in console #397
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
Comments
We could probably replace that line: return prefix + (schema.inputName || schema.label || schema.model || "") with this one: return prefix + (schema.inputName || schema.label || schema.model || Math.random().toString(36).substring(2, 15)) This will generate a random 11 char string as a last resort, if nothing else if available. You could also hash the schema object if you wanted something stable - this will get you a stable integer that will be unique if the schema object is unique - but will generate the same number when passed the same schema object: hashCode () {
var hash = 0;
for (var i = 0; i < this.length; i++) {
var character = this.charCodeAt(i);
hash = ((hash<<5)-hash)+character;
hash = hash & hash;
}
return Math.abs(hash);
}
...
return prefix + (schema.inputName || schema.label || schema.model || hashCode(schema)) |
Sorry for the drive-by - I don't currently have time to branch, test, etc... |
What if we replaced the slugifyFormID call with a call to Lodash's uniqueId ... I can't think of any reason why the ID's on elements shouldn't be randomly generated. We can provide logical prefixes, to help with debugging ... but, there is no reason for us to generate "known" ID values, if they are being used for styling, you can just use the various options for adding CSS classes to the elements. We can replace slugifyFormID with something like this:
|
This is what the lodash uniqueid function actually does: /**
* Generates a unique ID. If `prefix` is given, the ID is appended to it.
*
* @static
* @since 0.1.0
* @memberOf _
* @category Util
* @param {string} [prefix=''] The value to prefix the ID with.
* @returns {string} Returns the unique ID.
* @example
*
* _.uniqueId('contact_');
* // => 'contact_104'
*
* _.uniqueId();
* // => '105'
*/
function uniqueId(prefix) {
var id = ++idCounter;
return toString(prefix) + id;
} |
it's unique to every call ... so if it's called 100+ times, each call generates a unique ID from the previous, which provides us with legal "unique ID" values for HTML elements. We don't need fancy "prefix-h4h3h2h12h1" ID's ... VFG already relies on Lodash for various things, so reinventing the wheel here seems unnecessary. We can reduce the code in VFG, and eliminate an otherwise cumbersome/clunky looking block of code with a more elegant and simple approach. |
#468, #559 - added a "unique" flag to I believe VFG field ID's should all be uniquely generated, if the user wants them to contain "known" values they can pass the |
vue-form-generator/src/utils/schema.js
Line 65 in 1300882
Auto generating ID is not a good idea, specially since things like radio buttons share name. This creates errors.
Also,if no label or model, it will create fields with "" id, and if there are many, it will raise errors in the console.
Maybe skipping the implicit ID altogether would be better?
The text was updated successfully, but these errors were encountered: