From c89b0d4befaa50f8b6946741e9aeca659a0e288c Mon Sep 17 00:00:00 2001 From: Bruno Alves de Faria Date: Wed, 12 Apr 2017 13:42:00 -0300 Subject: [PATCH] Fixing validation issues in components --- .../client/components/base/mixins/form.js | 5 +- .../client/components/base/mixins/formItem.js | 47 ++++++++++++++----- .../client/components/base/mixins/group.js | 4 ++ .../components/bootstrap/dialog/info.html | 2 +- Packages/ohif-design/styles/common/form.styl | 3 ++ 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Packages/ohif-core/client/components/base/mixins/form.js b/Packages/ohif-core/client/components/base/mixins/form.js index 8603c07a5..c0e541f42 100644 --- a/Packages/ohif-core/client/components/base/mixins/form.js +++ b/Packages/ohif-core/client/components/base/mixins/form.js @@ -1,6 +1,7 @@ import { OHIF } from 'meteor/ohif:core'; import { Template } from 'meteor/templating'; import { Tracker } from 'meteor/tracker'; +import { Spacebars } from 'meteor/spacebars'; import { _ } from 'meteor/underscore'; import { $ } from 'meteor/jquery'; @@ -26,7 +27,7 @@ OHIF.mixins.form = new OHIF.Mixin({ instance.data.pathKey = ''; // Debound the observer call to prevent tons of re-rendering - component.validationRan = _.debounce(() => { + component.validationRan = _.throttle(() => { // Enable reactivity by changing a Tracker.Dependency observer component.validationObserver.changed(); }, 200); @@ -42,7 +43,7 @@ OHIF.mixins.form = new OHIF.Mixin({ // Focus the first error field if some validation failed if (component.schema && component.schema._invalidKeys.length) { - instance.$('.state-error :input:first').focus(); + Tracker.afterFlush(() => instance.$('.state-error :input:first').focus()); } return validationResult; diff --git a/Packages/ohif-core/client/components/base/mixins/formItem.js b/Packages/ohif-core/client/components/base/mixins/formItem.js index de5e228f6..1aae4f228 100644 --- a/Packages/ohif-core/client/components/base/mixins/formItem.js +++ b/Packages/ohif-core/client/components/base/mixins/formItem.js @@ -75,7 +75,7 @@ OHIF.mixins.formItem = new OHIF.Mixin({ // Set the component in error state and display the error message component.error = errorMessage => { // Set the component error state - component.state('error', errorMessage); + component.state('error', !!errorMessage); // Set or remove the error message if (errorMessage) { @@ -93,8 +93,8 @@ OHIF.mixins.formItem = new OHIF.Mixin({ return; } - // Create the tooltip - component.$wrapper.tooltip({ + // Destroy the tooltip if created and create again + component.$wrapper.tooltip('destroy').tooltip({ trigger: 'manual', title: message }).tooltip('show'); @@ -127,7 +127,7 @@ OHIF.mixins.formItem = new OHIF.Mixin({ // Show the tooltip with the error message component.toggleTooltip(true, errorMessage); } - }, + }; // Search for the parent form component component.getForm = () => { @@ -227,13 +227,42 @@ OHIF.mixins.formItem = new OHIF.Mixin({ // Add the pathKey to the wrapper element component.$wrapper.attr('data-key', instance.data.pathKey); + + // Get the component's form + const form = component.getForm(); + + // Observer for changes and revalidate the component + instance.autorun(computation => { + component.changeObserver.depend(); + + // Stop here if it is the first run + if (computation.firstRun) return; + + // Revalidate the component if form is already validated + if (form && form.isValidatedAlready) { + component.validate(); + } + }); }, onDestroyed() { const instance = Template.instance(); + const component = instance.component; - // Register the component in the parent component - instance.component.unregisterSelf(); + // Get the component's form for further use + const form = component.getForm(); + + // Unregister the component in the parent component + component.unregisterSelf(); + + // Remove the component tooltip, error state and message + component.error(false); + component.toggleTooltip(false); + + // Revalidate the form to remove this component from validation results + if (form && form.isValidatedAlready) { + form.validate(); + } }, onMixins() { @@ -260,12 +289,6 @@ OHIF.mixins.formItem = new OHIF.Mixin({ if (event.currentTarget === component.$element[0]) { // Enable reactivity by changing a Tracker.Dependency observer component.changeObserver.changed(); - - const form = component.getForm(); - if (form && form.isValidatedAlready) { - // Revalidate the component if form is already validated - component.validate(); - } } }, diff --git a/Packages/ohif-core/client/components/base/mixins/group.js b/Packages/ohif-core/client/components/base/mixins/group.js index 7bbe655b0..73a30e673 100644 --- a/Packages/ohif-core/client/components/base/mixins/group.js +++ b/Packages/ohif-core/client/components/base/mixins/group.js @@ -1,5 +1,6 @@ import { OHIF } from 'meteor/ohif:core'; import { Template } from 'meteor/templating'; +import { ReactiveVar } from 'meteor/reactive-var'; import { _ } from 'meteor/underscore'; /* @@ -111,6 +112,9 @@ OHIF.mixins.group = new OHIF.Mixin({ return result; } + // Reset the validation + component.schema.resetValidation(); + // Validate the component itself if it has a key if (instance.data.pathKey && !validateSelf()) { result = false; diff --git a/Packages/ohif-core/client/components/bootstrap/dialog/info.html b/Packages/ohif-core/client/components/bootstrap/dialog/info.html index 10aa35ad5..52b355e5c 100644 --- a/Packages/ohif-core/client/components/bootstrap/dialog/info.html +++ b/Packages/ohif-core/client/components/bootstrap/dialog/info.html @@ -6,7 +6,7 @@ {{#each message in this.messages}}
{{{message}}}
{{else}} - {{#let message=(choose this.message 'An error has ocurred.')}} + {{#let message=(choose this.reason this.message 'An error has ocurred.')}}
{{message}}
{{/let}} {{/each}} diff --git a/Packages/ohif-design/styles/common/form.styl b/Packages/ohif-design/styles/common/form.styl index 1ed73ed10..a9c1b4f63 100644 --- a/Packages/ohif-design/styles/common/form.styl +++ b/Packages/ohif-design/styles/common/form.styl @@ -2,6 +2,9 @@ label.wrapperLabel cursor: pointer + +label.wrapperLabel:not(.checkboxLabel) + cursor: pointer display: flex flex-direction: column