Rework and html for label & hint #467

lionel-bijaoui · 2018-06-20T09:56:24Z

Please check if the PR fulfills these requirements

The commit message follows our guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature: Allow for HTML in the label and hint field

What is the current behavior? (You can also link to an open issue here)
Text only for label and hint

What is the new behavior (if this is a feature change)?
Full html for label and hint

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No

Other information:

Build on top of the v3 branch

This work is following the changes introduced in my latest PR and should be merged only after that.
If my PR #465 is rejected, I will cherry-pick the necessary commit, deal with the conflicts and remake this PR. This will take me time for nothing, so I hope #465 will be accepted and merged.
I did that to simplify the work for the v3 features and rework.
By introducing some of these changes right now, I hope to prepare the code for the future.

New component

Part of formGenerator.vue was put inside another component. The necessary changes were made and the test changed to fit the new structure.
It should be almost transparent to the user, except if they used complicated way to access the fields from <vue-form-components> (eg. with ref like this.$refs.vfg.children[0]). And even then, I tried to make it as transparent as possible.

Doing without Pug

As you may notice, this PR is not using Pug for the new component.
I propose we stop using Pug in the future and that we slowly remove it.
Here are the reason why :

ESlint doesn't work in Pug template, so we get no lintting from eslint-plugin-vue
Pug can be a barrier to entry to collaborator as they need to learn (or know) this specific syntax to help
Pug is not bringing new functionnlity/solution (vue template are dynamic enough and Emmet is a thing)
To use Pug, we need additionnal plugin/parser/linter to be installed (can introduce problem/incompatibility)

…problems in fieldCheckbox.spec tests related to the updates ...

…ling, removed "checkbox" test from fieldInput,

…ages for each field to allow for more extensive testing, examples and documentation on each

* master: fix bower.json validation updated tests for modified label logic don't render labels when no label text is provided, proposed option 1 from vue-generators#347

* feature/345-debounced-validate-fix: remove uniqueId import fixes vue-generators#345 - declare debouncedValidateFunc and set it when debouncedValidate() is called... vue 2.2.0 prevents you from attaching methods/properties to components that have not been declared

* feature/345-debounced-validate-fix: removed console.log and fixed quotes

update to vue 2.5.9

…n of the config files. Now use Prettier, so most files are will need to be updated to validate. This is stil an early WIP. Known bugs: validation is broken since rewritten in without "module.exports"

# Conflicts: # dist/vfg-core.css # dist/vfg-core.js # dist/vfg.css # dist/vfg.js # package-lock.json # package.json # src/fields/abstractField.js # src/fields/core/fieldInput.vue # src/fields/core/fieldSelect.vue # src/fields/core/fieldSubmit.vue # src/formGenerator.vue # src/utils/validators.js # test/unit/specs/util.js

Unit test

Update to Webpack 3. Almost all dependency are updated.

…tests

…upport for other named sets) attributes to elements to support dynamic attributes (such as `title="Tooltip Text" data-toggle="tooltip"`, etc) * added directive to abstractField * implemented directive in various core fields as an example of usage * updated basic/app.vue with samples

V3: updated abstractField and fieldImage to clean up test errors

* v3: added brackets to conditional cleaned up the "clean up"

…ic-html-attributes V3: dynamic html attributes

# Conflicts: # .travis.yml # dist/vfg-core.css # dist/vfg-core.js # dist/vfg.css # dist/vfg.js # package-lock.json # package.json # src/fields/core/fieldInput.vue # src/fields/core/fieldSubmit.vue # src/fields/core/fieldUpload.vue # src/fields/optional/fieldSwitch.vue # src/formGenerator.vue

V3 update

Allow HTML for label & hint

dflock · 2018-06-25T20:05:20Z

build/webpack.build.config.js

+	" * vue-form-generator v" +
+	version +
+	"\n" +
+	" * https://github.com/icebob/vue-form-generator\n" +


This is the old URL - it's now here: https://github.com/vue-generators/vue-form-generator/

dflock · 2018-06-25T20:39:10Z

dev/mixinUtils.js

-
-	filters: {
-		prettyJSON: function(json) {
+export default {


I would probably call this file dev/mixins/utils.js

I'm changing that no problem

dflock · 2018-06-25T20:42:48Z

package.json

+	],
+	"repository": {
+		"type": "git",
+		"url": "https://github.com/icebob/vue-form-generator.git"


This one is wrong too.

dflock · 2018-06-25T20:49:35Z

package.json

+		"singleQuote": false,
+		"trailingComma": "none",
+		"bracketSpacing": true,
+		"semi": true,


I know the existing codebase is like this, but it'd be nice at some point to remove some of these Prettier options - not now, though :)

Why remove the options ?

Personally prefer less indentation and less semicolons ¯\_(ツ)_/¯

With Prettier, everything is done automatically. Even if you write your code a certain way, Prettier format it to this way on save (add semicolons, fix indentation). You don't have to change your habits and it allow for an unified way of writing.
I based myself on the existing code. The indentation is arbitrary, but I have a strong opinion about semicolons. A missing semicolons can cause a bug in some cases, too much semicolons cannot. Also, while parser can understand js without semi, it is not the recommended way to write js (https://stackoverflow.com/questions/444080/do-you-recommend-using-semicolons-after-every-statement-in-javascript).

I’d rather there be too many semis than not enough. /opinion

dflock · 2018-06-25T21:09:27Z

src/fields/optional/fieldNoUiSlider.vue

@@ -1,116 +1,117 @@
 <template>
-	<div class="slider" :disabled="disabled" :class="{ 'contain-pips': containPips, 'contain-tooltip': containTooltip }"></div>  
+	<div class="slider" :disabled="disabled" :class="{ 'contain-pips': containPips, 'contain-tooltip': containTooltip }"></div>


This not getting v-attributes?

I didn't think about it. Since fields will be separated from the main project (in the v3), I'm not touching them too much for now.

dflock · 2018-06-25T21:15:53Z

src/utils/schema.js

 			.toString()
 			.trim()
-			.toLowerCase()
+			// .toLowerCase()


Why comment this line out?

It come from that 17aba74
I'm not going to change it

dflock · 2018-06-25T21:16:55Z

src/utils/schema.js

+			// .toLowerCase()
 			// Spaces & underscores to dashes
-			.replace(/ |_/g, "-")
+			.replace(/ /g, "-")


If you're going to change the output & meaning, change the comments to match.

Again, it come from 17aba74
I'm going to make the change you suggested

dflock · 2018-06-25T21:17:13Z

src/utils/schema.js

 			.replace(/-{2,}/g, "-")
 			// Remove leading & trailing dashes
 			.replace(/^-+|-+$/g, "")
 			// Remove anything that isn't a (English/ASCII) letter, number or dash.


If you're going to change the output & meaning, change the comments to match.

Again, it come from 17aba74
I'm going to make the change you suggested

dflock · 2018-06-25T21:19:28Z

src/utils/schema.js

 		// Return the slugified version of either:
-		return prefix + (schema.inputName || schema.label || schema.model || "")
+		return (
+			prefix +


Are you duplicating the slugify function in here?

Again, it come from 17aba74, I have no idea why there is two slightly different slugify...

zoul0813 · 2018-06-26T12:56:12Z

package.json

+	"name": "vue-form-generator",
+	"version": "3.0.0-dev",
+	"description": "A schema-based form generator component for Vue.js",
+	"homepage": "https://github.com/icebob/vue-form-generator",


https://github.com/vue-generators/vue-form-generator/

Yes, sorry, I'm going to fix that

zoul0813 · 2018-06-26T13:01:16Z

src/utils/schema.js

+		return (
+			prefix +
+			(schema.inputName || schema.label || schema.model || "")
+				// NB: This is a very simple, conservative, slugify function,


we already rely on lodash, why not just use _.kebabCase() here so it's a bit clearer and shorter?

Again, I'm sorry, I have no idea why we do that. This is a good suggestion but I'm not going to implement too much changes in this PR (there is already a lot of changes). Maybe next PR ? Or v3 ?

Sounds good - we should probably do a full code review at some point and simplify whatever we can, relying on known functions with clear documentation (such as kebabCase).

You are absolutely right !

lionel-bijaoui · 2018-06-26T13:38:00Z

@dflock @zoul0813 Thank you for your feedback. Is it better now ?

zoul0813 · 2018-06-26T14:03:57Z

This is backwards compatible with v2 correct? Should the version be 2.2.3 or 2.3.0 ... instead of 3.0.0-dev? Sorry, missed that during the initial review.

zoul0813 · 2018-06-26T14:06:04Z

package.json

@@ -1,8 +1,8 @@
 {
 	"name": "vue-form-generator",
 	"version": "3.0.0-dev",


should be 2.2.3 or 2.30 - I'm thinking 2.3.0 due to all the changes?

Yes, sorry. It was the v3 branch initially but I actually only made backward compatible changes. So I will call it 2.3.0, it will require npm i for all the collaborator since the test and plugin are updated, but it will change nothing for the end user. I tested it on my project without any problem and I use fairly complex custom fields, an api to generate the schema, complex dynamic functionality and they needed no changes and introduce no bug.
I might have missed something of course, but I'm pretty confident.

lionel-bijaoui · 2018-06-26T17:04:38Z

@dflock That should be good

dflock

lgtm!

zoul0813 and others added 30 commits December 1, 2017 17:40

updated to latest npm packages, added new "basic" dev samples, fixed …

a18f608

…problems in fieldCheckbox.spec tests related to the updates ...

updated core field tests to use modified nextTick with exception hand…

a1f31d3

…ling, removed "checkbox" test from fieldInput,

skipping optional fields, proposing these be moved into separate pack…

2e192ae

…ages for each field to allow for more extensive testing, examples and documentation on each

disabled "color" test for fieldInput.spec to enable build

cf90bd6

Merge branch 'master' into feature/vue-2.5.9

1cb15d8

* master: fix bower.json validation updated tests for modified label logic don't render labels when no label text is provided, proposed option 1 from vue-generators#347

Merge branch 'feature/345-debounced-validate-fix' into feature/vue-2.5.9

dc9c88f

* feature/345-debounced-validate-fix: removed console.log and fixed quotes

build

55dff90

Merge pull request vue-generators#355 from zoul0813/feature/vue-2.5.9

244abb9

update to vue 2.5.9

Update to Webpack 3. Almost all dependency are updated. Reorganisatio…

163fdc9

…n of the config files. Now use Prettier, so most files are will need to be updated to validate. This is stil an early WIP. Known bugs: validation is broken since rewritten in without "module.exports"

Update configuration and dependency

49043ab

Fix and correction to pass Eslint and other checks

056d9db

WIP on the unit test

e97420e

WIP on unit test

2d74a15

Dependency updated

ebf2202

Progress on test

166ee43

Progress on test

59aaf2b

Work on the code coverage

a858da6

Merge pull request #2 from lionel-bijaoui/unit_test

057a1ad

Unit test

Progress on test

49c789c

Cleaning of the test

6bf1cd2

Merge pull request #3 from lionel-bijaoui/unit_test

4498e6b

Unit test

Correction of build and small corrections

632911e

Remove config file

e932b58

Merge pull request vue-generators#357 from lionel-bijaoui/v3

64d0421

Update to Webpack 3. Almost all dependency are updated.

updated abstractField and fieldImage to clean up errors generated by …

4b7f551

…tests

added tests and fixed v-attributes binding on components

e7859b7

cleaned up the "clean up"

3594edf

zoul0813 and others added 8 commits March 14, 2018 10:38

added brackets to conditional

3249775

Merge pull request vue-generators#414 from zoul0813/v3

70d6798

V3: updated abstractField and fieldImage to clean up test errors

Merge branch 'v3' into v3-features/dynamic-html-attributes

5fdf339

* v3: added brackets to conditional cleaned up the "clean up"

Merge pull request vue-generators#415 from zoul0813/v3-features/dynam…

e21a0de

…ic-html-attributes V3: dynamic html attributes

Fix test and update dependency

b9ea983

Merge pull request #4 from lionel-bijaoui/v3_update

26960e6

V3 update

Rework of formGenerator to use a component

3f52a20

Allow HTML for label & hint

lionel-bijaoui requested review from cristijora, dflock, icebob and zoul0813 and removed request for icebob and zoul0813 June 20, 2018 09:57

dflock reviewed Jun 25, 2018

View reviewed changes

zoul0813 reviewed Jun 26, 2018

View reviewed changes

Lionel Bijaoui and others added 2 commits June 26, 2018 15:33

Fix URL, rename and move mixinsUtils.js, modify a comment

90c4673

Merge branch 'master' into Rework_and_HTML_for_label_hint

aaaa3c0

zoul0813 reviewed Jun 26, 2018

View reviewed changes

Fix version number

e061755

zoul0813 approved these changes Jun 26, 2018

View reviewed changes

dflock approved these changes Jun 26, 2018

View reviewed changes

lionel-bijaoui merged commit bc44da9 into vue-generators:master Jun 27, 2018

lionel-bijaoui deleted the Rework_and_HTML_for_label_hint branch June 27, 2018 08:04

Rework and html for label & hint #467

Rework and html for label & hint #467

Uh oh!

Conversation

lionel-bijaoui commented Jun 20, 2018 • edited by dflock Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build on top of the v3 branch

New component

Doing without Pug

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

lionel-bijaoui Jun 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lionel-bijaoui Jun 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lionel-bijaoui Jun 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lionel-bijaoui commented Jun 26, 2018

Uh oh!

zoul0813 commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lionel-bijaoui commented Jun 26, 2018

Uh oh!

lionel-bijaoui commented Jun 20, 2018 •

edited by dflock

Loading

lionel-bijaoui Jun 26, 2018 •

edited

Loading

lionel-bijaoui Jun 26, 2018 •

edited

Loading

lionel-bijaoui Jun 26, 2018 •

edited

Loading

dflock left a comment •

edited

Loading