Skip to content
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

FIO-9329: validateWhenHidden respects both conditionally hidden and intentionally hidden #5904

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"dependencies": {
"@formio/bootstrap": "3.0.0-dev.98.17ba6ea",
"@formio/choices.js": "^10.2.1",
"@formio/core": "v2.1.0-dev.174.9a3c6ec",
"@formio/core": "2.1.0-dev.191.8c609ab",
"@formio/text-mask-addons": "^3.8.0-formio.3",
"@formio/vanilla-text-mask": "^5.1.1-formio.1",
"abortcontroller-polyfill": "^1.7.5",
Expand Down
84 changes: 61 additions & 23 deletions src/components/_classes/component/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,19 @@
// Needs for Nextgen Rules Engine
this.resetCaches();

/**
* Determines if this component is conditionally hidden. Should generally not be set outside of conditional logic pipeline.
* This is necessary because of clearOnHide behavior that only clears when conditionally hidden - we need to track
* conditionallyHidden separately from "regular" visibility.
*/
this._parentConditionallyHidden = this.options.hasOwnProperty('parentConditionallyHidden') ? this.options.parentConditionallyHidden : false;
this._conditionallyHidden = this.checkConditionallyHidden(null, data) || this._parentConditionallyHidden;

/**
* Determines if this component is visible, or not.
*/
this._parentVisible = this.options.hasOwnProperty('parentVisible') ? this.options.parentVisible : true;
this._visible = this._parentVisible && this.conditionallyVisible(null, data);
this._visible = this._parentVisible && (this.hasCondition() ? !this._conditionallyHidden : !this.component.hidden);
this._parentDisabled = false;

/**
Expand Down Expand Up @@ -447,7 +455,7 @@
if (this.allowData && this.key) {
this.options.name += `[${this.key}]`;
// If component is visible or not set to clear on hide, set the default value.
if (this.visible || !this.component.clearOnHide) {
if (!(this.conditionallyHidden && this.component.clearOnHide)) {
if (!this.hasValue()) {
if (this.shouldAddDefaultValue) {
this.dataValue = this.defaultValue;
Expand Down Expand Up @@ -535,7 +543,8 @@

init() {
this.disabled = this.shouldDisabled;
this._visible = this.conditionallyVisible(null, null);
this._conditionallyHidden = this.checkConditionallyHidden();
this._visible = (this.hasCondition() ? !this.conditionallyHidden : !this.component.hidden);
if (this.component.addons?.length) {
this.component.addons.forEach((addon) => this.createAddon(addon));
}
Expand Down Expand Up @@ -675,7 +684,6 @@
return;
}
this._visible = value;
this.clearOnHide();
this.redraw();
}
}
Expand All @@ -698,6 +706,23 @@
return this._visible && this._parentVisible;
}

get conditionallyHidden() {
return this._conditionallyHidden || this._parentConditionallyHidden;
}

/**
* Evaluates whether the component is conditionally hidden (as opposed to intentionally hidden, e.g. via the `hidden` component schema property).
* @param {object} data - The data object to evaluate the condition against.
* @param {object} row - The row object to evaluate the condition against.
* @returns {boolean} - Whether the component is conditionally hidden.
*/
checkConditionallyHidden(data = null, row = null) {
if (!this.hasCondition()) {
return false;
}
return !this.conditionallyVisible(data, row);
}

get currentForm() {
return this._currentForm;
}
Expand Down Expand Up @@ -2028,7 +2053,7 @@
rebuild() {
this.destroy();
this.init();
this.visible = this.conditionallyVisible(null, null);
this.visible = this.hasCondition() ? !this.conditionallyHidden : !this.component.hidden;
return this.redraw();
}

Expand Down Expand Up @@ -2105,8 +2130,8 @@
conditionallyVisible(data, row) {
data = data || this.rootValue;
row = row || this.data;
if (this.builderMode || this.previewMode || !this.hasCondition()) {
return !this.component.hidden;
if (this.builderMode || this.previewMode) {
return true;
}
data = data || (this.root ? this.root.data : {});
return this.checkCondition(row, data);
Expand Down Expand Up @@ -2146,8 +2171,15 @@
this.redraw();
}

// Check advanced conditions
const visible = this.conditionallyVisible(data, row);
// Check advanced conditions (and cache the result)
const isConditionallyHidden = this.checkConditionallyHidden(data, row) || this._parentConditionallyHidden;
if (isConditionallyHidden !== this._conditionallyHidden) {
this._conditionallyHidden = isConditionallyHidden;
this.clearOnHide();
}

// Check visibility
const visible = (this.hasCondition() ? !this.conditionallyHidden : !this.component.hidden);

if (this.visible !== visible) {
this.visible = visible;
Expand Down Expand Up @@ -2289,6 +2321,12 @@

const property = action.property.value;
if (!_.isEqual(_.get(this.component, property), _.get(newComponent, property))) {
// Advanced Logic can modify the component's hidden property; because we track conditionally hidden state
// separately from the component's hidden property, and technically this Advanced Logic conditionally hides
// a component, we need to set _conditionallyHidden to the new value
if (property === 'hidden') {
this._conditionallyHidden = newComponent.hidden;
}
changed = true;
}

Expand All @@ -2307,7 +2345,7 @@
}
);

if (!_.isEqual(oldValue, newValue) && !(this.component.clearOnHide && !this.visible)) {
if (!_.isEqual(oldValue, newValue) && !(this.component.clearOnHide && this.conditionallyHidden)) {
this.setValue(newValue);

if (this.viewOnly) {
Expand Down Expand Up @@ -2352,7 +2390,7 @@
},
'value');

if (!_.isEqual(oldValue, newValue) && !(this.component.clearOnHide && !this.visible)) {
if (!_.isEqual(oldValue, newValue) && !(this.component.clearOnHide && this.conditionallyHidden)) {
this.setValue(newValue);

if (this.viewOnly) {
Expand Down Expand Up @@ -2481,7 +2519,7 @@
!this.options.readOnly &&
!this.options.showHiddenFields
) {
if (!this.visible) {
if (this.conditionallyHidden) {
this.deleteValue();
}
else if (!this.hasValue() && this.shouldAddDefaultValue) {
Expand Down Expand Up @@ -2776,7 +2814,7 @@
get dataValue() {
if (
!this.key ||
(!this.visible && this.component.clearOnHide && !this.rootPristine)
(this.conditionallyHidden && this.component.clearOnHide && !this.rootPristine)
) {
return this.emptyValue;
}
Expand All @@ -2798,7 +2836,7 @@
if (
!this.allowData ||
!this.key ||
(!this.visible && this.component.clearOnHide && !this.rootPristine)
(this.conditionallyHidden && this.component.clearOnHide && !this.rootPristine)
) {
return;
}
Expand Down Expand Up @@ -2969,7 +3007,7 @@

/**
* Returns if the value (e.g. array) should be divided between several inputs
* @returns {boolean}

Check warning on line 3010 in src/components/_classes/component/Component.js

View workflow job for this annotation

GitHub Actions / setup

Missing JSDoc @returns description
*/
isSingleInputValue() {
return false;
Expand Down Expand Up @@ -3162,7 +3200,7 @@
// If no calculated value or
// hidden and set to clearOnHide (Don't calculate a value for a hidden field set to clear when hidden)
const { clearOnHide } = this.component;
const shouldBeCleared = !this.visible && clearOnHide;
const shouldBeCleared = this.conditionallyHidden && clearOnHide;
const allowOverride = _.get(this.component, 'allowCalculateOverride', false);

if (shouldBeCleared) {
Expand Down Expand Up @@ -3706,12 +3744,6 @@
}

shouldSkipValidation(data, row, flags = {}) {
const { validateWhenHidden = false } = this.component || {};
const forceValidOnHidden = (!this.visible || !this.checkCondition(row, data)) && !validateWhenHidden;
if (forceValidOnHidden) {
// If this component is forced valid when it is hidden, then we also need to reset the errors for this component.
this._errors = [];
}
const rules = [
// Do not validate if the flags say not too.
() => flags.noValidate,
Expand All @@ -3722,7 +3754,13 @@
// Check to see if we are editing and if so, check component persistence.
() => this.isValueHidden(),
// Force valid if component is hidden.
() => forceValidOnHidden
() => {
if (!this.component.validateWhenHidden && (!this.visible || !this.checkCondition(row, data))) {
this._errors = [];
return true;
}
return false;
}
];

return rules.some(pred => pred());
Expand Down Expand Up @@ -3877,7 +3915,7 @@
// If component definition changed, replace it.
if (!_.isEqual(this.component, newComponent)) {
this.component = newComponent;
const visible = this.conditionallyVisible(null, null);
const visible = this.hasCondition() ? !this.conditionallyHidden : !this.component.hidden;
const disabled = this.shouldDisabled;

// Change states which won't be recalculated during redrawing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ export default [
{
weight: 700,
type: 'checkbox',
label: 'Clear Value When Hidden',
label: 'Omit Value From Submission Data When Conditionally Hidden',
key: 'clearOnHide',
defaultValue: true,
tooltip: 'When a field is hidden, clear the value.',
tooltip: 'When a field is conditionally hidden, omit the value from the submission data.',
input: true
},
EditFormUtils.javaScriptValue('Custom Default Value', 'customDefaultValue', 'customDefaultValue', 1000,
Expand Down
24 changes: 17 additions & 7 deletions src/components/_classes/nested/NestedComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,27 +86,36 @@ export default class NestedComponent extends Field {
const visibilityChanged = this._visible !== value;
this._visible = value;
const isVisible = this.visible;
const isConditionallyHidden = this.checkConditionallyHidden();
const forceShow = this.shouldForceShow();
const forceHide = this.shouldForceHide();
this.components.forEach(component => {
this.components.forEach((component) => {
// Set the parent visibility first since we may have nested components within nested components
// and they need to be able to determine their visibility based on the parent visibility.
component.parentVisible = isVisible;
component._parentConditionallyHidden = isConditionallyHidden;
let visible;
if (component.hasCondition()) {
component._conditionallyHidden = component.checkConditionallyHidden() || component._parentConditionallyHidden;
visible = !component.conditionallyHidden;
}
else {
visible = !component.component.hidden;
}

const conditionallyVisible = component.conditionallyVisible();
if (forceShow || conditionallyVisible) {
if (forceShow || visible) {
component.visible = true;
}
else if (forceHide || !isVisible || !conditionallyVisible) {
else if (forceHide || !isVisible || !visible ) {
component.visible = false;
}
// If hiding a nested component, clear all errors below.
if (!component.visible) {
component.error = '';
}
});

if (visibilityChanged) {
this.clearOnHide();
this.redraw();
}
}
Expand Down Expand Up @@ -421,6 +430,7 @@ export default class NestedComponent extends Field {
data = data || this.data;
options.parent = this;
options.parentVisible = this.visible;
options.parentConditionallyHidden = this.conditionallyHidden;
options.root = options?.root || this.root || this;
options.localRoot = this.localRoot;
options.skipInit = true;
Expand Down Expand Up @@ -710,7 +720,7 @@ export default class NestedComponent extends Field {
clearOnHide(show) {
super.clearOnHide(show);
if (this.component.clearOnHide) {
if (this.allowData && !this.hasValue() && !(this.options.server && !this.visible)) {
if (this.allowData && !this.hasValue() && !this.conditionallyHidden) {
this.dataValue = this.defaultValue;
}
if (this.hasValue()) {
Expand Down Expand Up @@ -743,7 +753,7 @@ export default class NestedComponent extends Field {

calculateValue(data, flags, row) {
// Do not iterate into children and calculateValues if this nested component is conditionally hidden.
if (!this.conditionallyVisible()) {
if (this.conditionallyHidden) {
return false;
}
return this.getComponents().reduce(
Expand Down
2 changes: 1 addition & 1 deletion src/components/datamap/DataMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export default class DataMapComponent extends DataGridComponent {
get dataValue() {
if (
!this.key ||
(!this.visible && this.component.clearOnHide)
(this.conditionallyHidden && this.component.clearOnHide)
) {
return this.emptyValue;
}
Expand Down
7 changes: 2 additions & 5 deletions src/components/editgrid/EditGrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,7 @@ export default class EditGridComponent extends NestedArrayComponent {
}

const changed = this.hasChanged(value, this.dataValue);
if (this.parent && !this.options.server) {
if (this.parent) {
this.parent.checkComponentConditions();
}
this.dataValue = value;
Expand Down Expand Up @@ -1383,10 +1383,7 @@ export default class EditGridComponent extends NestedArrayComponent {

this.openWhenEmpty();
this.updateOnChange(flags, changed);
// do not call checkData with server option, it is called when change is triggered in updateOnChange
if (!this.options.server) {
this.checkData();
}
this.checkData();

this.changeState(changed, flags);

Expand Down
8 changes: 4 additions & 4 deletions src/components/form/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -475,11 +475,11 @@ export default class FormComponent extends Component {
}

hideSubmitButton(component) {
const isSubmitButton = (component.type === 'button') &&
((component.action === 'submit') || !component.action);
const isSubmitButton = component.type === 'button' && (component.action === 'submit' || !component.action);

if (isSubmitButton) {
component.hidden = true;
component.customConditional = 'show = false';
}
}

Expand All @@ -489,7 +489,7 @@ export default class FormComponent extends Component {
* @returns {Promise} - The promise that resolves when the subform is loaded.
*/
loadSubForm(fromAttach) {
if (this.builderMode || this.isHidden() || (this.isSubFormLazyLoad() && !fromAttach)) {
if (this.builderMode || this.conditionallyHidden || (this.isSubFormLazyLoad() && !fromAttach)) {
return Promise.resolve();
}

Expand Down Expand Up @@ -586,7 +586,7 @@ export default class FormComponent extends Component {
* @returns {*|boolean} - TRUE if the subform should be submitted, FALSE if it should not.
*/
get shouldSubmit() {
return this.subFormReady && (!this.component.hasOwnProperty('reference') || this.component.reference) && !this.isHidden();
return this.subFormReady && (!this.component.hasOwnProperty('reference') || this.component.reference) && !this.conditionallyHidden;
}

/**
Expand Down
19 changes: 16 additions & 3 deletions src/components/html/HTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,22 @@ export default class HTMLComponent extends Component {

checkRefreshOn(changed) {
super.checkRefreshOn(changed);
if (!this.builderMode && this.component.refreshOnChange && this.element &&
!_.isUndefined(changed) && ((_.isBoolean(changed) && changed) || !_.isEmpty(changed)) &&
this.conditionallyVisible(this.data, this.row)) {
let visible;
if (this.hasCondition()) {
this._conditionallyHidden = this.checkConditionallyHidden();
visible = !this.conditionallyHidden;
}
else {
visible = !this.component.hidden;
}
const shouldSetContent = !this.builderMode
&& this.component.refreshOnChange
&& this.element
&& !_.isUndefined(changed)
&& ((_.isBoolean(changed) && changed) || !_.isEmpty(changed))
&& visible;

if (shouldSetContent) {
this.setContent(this.element, this.renderContent());
}
}
Expand Down
Loading
Loading