Skip to content

Commit

Permalink
Fixed #4889 - The 'Cannot read properties of undefined (reading 'valu…
Browse files Browse the repository at this point in the history
…e')' exception is thrown when using the Ctrl+Z to reset a calculated field name to the initial value (#4902)

* Fixed #4889 - The 'Cannot read properties of undefined (reading 'value')' exception is thrown when using the Ctrl+Z to reset a calculated field name to the initial value

* Fixed #4889 - The 'Cannot read properties of undefined (reading 'value')' exception is thrown when using the Ctrl+Z to reset a calculated field name to the initial value

* Fixed linter

* Fixed u-tests

---------

Co-authored-by: tsv2013 <[email protected]>
  • Loading branch information
tsv2013 and tsv2013 authored Nov 21, 2023
1 parent 443e8e9 commit a242b63
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 30 deletions.
13 changes: 10 additions & 3 deletions packages/survey-creator-core/src/creator-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3021,7 +3021,8 @@ export class CreatorBase extends Base
rootNode.removeEventListener("keydown", this.onKeyDownHandler);
}
}
protected onKeyDownHandler = (event: KeyboardEvent) => {
public findSuitableShortcuts(event: KeyboardEvent): IKeyboardShortcut[] {
const shortcuts: IKeyboardShortcut[] = [];
const availableShortcuts = Object.keys(this.shortcuts || {})
.map((key) => this.shortcuts[key])
.filter((shortcut: IKeyboardShortcut) => !shortcut.affectedTab || shortcut.affectedTab === this.activeTab);
Expand All @@ -3033,8 +3034,14 @@ export class CreatorBase extends Base
if (!!hotKey.ctrlKey !== !!event.ctrlKey) return;
if (!!hotKey.shiftKey !== !!event.shiftKey) return;
if (hotKey.keyCode !== event.keyCode) return;
if (hotKey.keyCode < 48 && isTextInput(event.target)) return;
shortcut.execute(this.selectElement);
shortcuts.push(shortcut);
});
return shortcuts;
}
protected onKeyDownHandler = (event: KeyboardEvent) => {
this.findSuitableShortcuts(event).forEach((shortcut: IKeyboardShortcut) => {
if ((event.keyCode < 48 || event.keyCode == 89 || event.keyCode == 90) && isTextInput(event.target)) return;
shortcut.execute(this.selectedElement);
});
}
private shortcuts: { [index: string]: IKeyboardShortcut } = {};
Expand Down
53 changes: 34 additions & 19 deletions packages/survey-creator-core/src/property-grid/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ import { QuestionLinkValueModel } from "../components/link-value";
import { CreatorBase } from "../creator-base";

function propertyVisibleIf(params: any): boolean {
if(!this.question) return false;
if (!this.question) return false;
const obj = this.question.obj;
const prop = this.question.property;
if (!obj || !prop) return false;
if(!Serializer.hasOriginalProperty(obj, prop.name)) return false;
if (!Serializer.hasOriginalProperty(obj, prop.name)) return false;
return prop.visibleIf(obj);
}
function propertyEnableIf(params: any): boolean {
Expand Down Expand Up @@ -514,11 +514,11 @@ export class PropertyJSONGenerator {
if (!!prop.visibleIf && eventVisibility) {
q.visibleIf = "propertyVisibleIf() = true";
}
if(!!prop.overridingProperty && q.visible) {
if (!!prop.overridingProperty && q.visible) {
q.onUpdateCssClassesCallback = (css: any) => {
css.questionWrapper = "spg-boolean-wrapper--overriding";
};
if(!eventReadOnly) {
if (!eventReadOnly) {
q.enableIf = "propertyEnableIf() = true";
}
const overridingQuestion = this.createOverridingQuestion(panel, q, prop.overridingProperty);
Expand Down Expand Up @@ -568,7 +568,7 @@ export class PropertyJSONGenerator {
linkValue.onUpdateCssClassesCallback = (css: any) => {
css.questionWrapper = "spg-link-wrapper--overriding";
};
if(!!overridingQuestion) {
if (!!overridingQuestion) {
linkValue.linkClickCallback = () => {
//Focus and aways scroll into view
overridingQuestion.focus(false, true);
Expand Down Expand Up @@ -757,7 +757,7 @@ export class PropertyGridModel {
options: ISurveyCreatorOptions = new EmptySurveyCreatorOptions()
) {
this.options = options;
if(this.options.enableLinkFileEditor) {
if (this.options.enableLinkFileEditor) {
PropertyGridEditorCollection.register(new PropertyGridLinkEditor());
}
this.obj = obj;
Expand Down Expand Up @@ -965,15 +965,15 @@ export class PropertyGridModel {
if (question.isRequired && (Helpers.isValueEmpty(val) || question["valueChangingEmpty"]))
return this.getErrorTextOnValidate(
editorLocalization.getString("pe.propertyIsEmpty"), prop.name, obj, val);
if(this.isPropNameInValid(obj, prop, val) || question["nameHasError"])
if (this.isPropNameInValid(obj, prop, val) || question["nameHasError"])
return this.getErrorTextOnValidate(
editorLocalization.getString("pe.propertyNameIsIncorrect"), prop.name, obj, val);
const editorError = PropertyGridEditorCollection.validateValue(obj, question, prop, val);
return this.getErrorTextOnValidate(editorError, prop.name, obj, val);
}
private getErrorTextOnValidate(defaultError: string, propName: string, obj: Base, val: any): string {
const customError = this.options.onGetErrorTextOnValidationCallback(propName, obj, val);
return !!customError ? customError: defaultError;
return !!customError ? customError : defaultError;
}
private onValidateQuestion(options: any) {
var q = options.question;
Expand All @@ -1000,7 +1000,7 @@ export class PropertyGridModel {
q["nameHasError"] = isPropertyNameInValid;
}
private isPropNameInValid(obj: Base, prop: JsonObjectProperty, val: any): boolean {
if(obj["isQuestion"] && prop.name === "name" && !!val) {
if (obj["isQuestion"] && prop.name === "name" && !!val) {
val = (<string>val).toLowerCase();
return ["item", "choice", "row", "panel"].indexOf(val) > -1;
}
Expand Down Expand Up @@ -1236,7 +1236,7 @@ export abstract class PropertyGridEditor implements IPropertyGridEditor {
if (property.type !== "condition") {
surveyPropertyEditor.editSurvey.css = defaultV2Css;
}
if(question.isReadOnly) {
if (question.isReadOnly) {
surveyPropertyEditor.editSurvey.mode = "display";
}
if (!settings.showDialog) return surveyPropertyEditor;
Expand Down Expand Up @@ -1265,7 +1265,7 @@ export abstract class PropertyGridEditor implements IPropertyGridEditor {
}, options.rootElement);
if (question.isReadOnly) {
const applyBtn = popupModel.footerToolbar.getActionById("apply");
if(!!applyBtn) {
if (!!applyBtn) {
applyBtn.visible = false;
}
}
Expand Down Expand Up @@ -1302,7 +1302,7 @@ export abstract class PropertyGridEditor implements IPropertyGridEditor {
}
onUpdateQuestionCssClasses(obj: Base, options: any) {
if (!this.isSupportGrouping()) return;
if(this.hasPreviousElementForGrouping(options.question)) {
if (this.hasPreviousElementForGrouping(options.question)) {
options.cssClasses.mainRoot += " spg-row-narrow__question";
}
}
Expand Down Expand Up @@ -1336,17 +1336,32 @@ export abstract class PropertyGridEditorStringBase extends PropertyGridEditor {
return json;
}
protected updateType(prop: JsonObjectProperty, obj: Base, json: any) {
if(!json.maxLength && obj.hasDefaultPropertyValue(prop.name)) {
if (!json.maxLength && obj.hasDefaultPropertyValue(prop.name)) {
json.type = `${json.type}withreset`;
}
return json;
}
public onCreated(obj: Base, question: Question, prop: JsonObjectProperty, options: ISurveyCreatorOptions) {
question.disableNativeUndoRedo = true;
if(prop.name === "title") {
if (question instanceof QuestionTextBase) {
question.onKeyDownPreprocess = (event: KeyboardEvent) => {
if ((event.ctrlKey || event.metaKey) && [89, 90].indexOf(event.keyCode) !== -1) {
if (question.isInputTextUpdate) {
(options as any).findSuitableShortcuts(event).forEach((shortcut: any) => {
shortcut.execute((options as any).selectedElement);
});
event.preventDefault();
} else if ((event.target as HTMLInputElement).value == question.value) {
(options as any).findSuitableShortcuts(event).forEach((shortcut: any) => {
shortcut.execute((options as any).selectedElement);
});
}
}
};
}
if (prop.name === "title") {
question.allowSpaceAsAnswer = true;
}
if(question.getType() == "textwithreset" || question.getType() == "commentwithreset") {
if (question.getType() == "textwithreset" || question.getType() == "commentwithreset") {
question.resetValueAdorner.resetValueCallback = () => {
obj.resetPropertyValue(prop.name);
};
Expand Down Expand Up @@ -1396,9 +1411,9 @@ export class PropertyGridLinkEditor extends PropertyGridEditor {
}

public onCreated(obj: Base, question: QuestionFileEditorModel, prop: JsonObjectProperty, options: ISurveyCreatorOptions) {
if(["image", "imageitemvalue"].indexOf(obj.getType()) > -1) {
if (["image", "imageitemvalue"].indexOf(obj.getType()) > -1) {
const questionObj = obj.getType() == "imageitemvalue" ? (<any>obj).locOwner : <Question>obj;
if(questionObj) {
if (questionObj) {
questionObj.registerFunctionOnPropertyValueChanged("contentMode", (newValue: string) => {
question.acceptedTypes = getAcceptedTypesByContentMode(newValue);
});
Expand Down Expand Up @@ -1459,7 +1474,7 @@ export class PropertyGridEditorImageSize extends PropertyGridEditorString {
const isDefaultValue = (imageHeight: number, imageWidth: number) => {
const imageHeightProperty = Serializer.findProperty(obj.getType(), "imageHeight");
const imageWidthProperty = Serializer.findProperty(obj.getType(), "imageWidth");
if(!imageHeightProperty && !imageWidthProperty) return false;
if (!imageHeightProperty && !imageWidthProperty) return false;
return imageHeightProperty.isDefaultValue(imageHeight) && imageWidthProperty.isDefaultValue(imageWidth);
};
question.valueFromDataCallback = function (value: any): any {
Expand Down
35 changes: 34 additions & 1 deletion packages/survey-creator-core/tests/creator-base.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2780,6 +2780,38 @@ test("process shortcut for text inputs", (): any => {
expect(log).toEqual("->execute->execute->execute");
});

test("process undo-redo shortcut for text inputs", (): any => {
const creator = new CreatorTester({ showDesignerTab: false });
let log = "";
creator.registerShortcut("undo_test", {
hotKey: {
keyCode: 90,
},
macOsHotkey: {
keyCode: 90,
},
execute: () => log += "->execute"
});
creator.registerShortcut("redo_test", {
hotKey: {
keyCode: 89,
},
macOsHotkey: {
keyCode: 89,
},
execute: () => log += "->execute"
});
expect(log).toEqual("");
creator["onKeyDownHandler"](<any>{ keyCode: 90, target: { tagName: "span" } });
expect(log).toEqual("->execute");
creator["onKeyDownHandler"](<any>{ keyCode: 89, target: { tagName: "div" } });
expect(log).toEqual("->execute->execute");
creator["onKeyDownHandler"](<any>{ keyCode: 89, target: { tagName: "input" } });
expect(log).toEqual("->execute->execute");
creator["onKeyDownHandler"](<any>{ keyCode: 90, target: { tagName: "input" } });
expect(log).toEqual("->execute->execute");
});

test("doClickQuestionCore", () => {
const creator = new CreatorTester({ showLogicTab: true });
creator.JSON = {
Expand Down Expand Up @@ -2976,7 +3008,8 @@ test("Add new question from Page on selecting question in panel dynamic", (): an
const creator = new CreatorTester();
creator.JSON = {
elements: [
{ type: "paneldynamic", name: "panel1",
{
type: "paneldynamic", name: "panel1",
templateElements: [
{ type: "text", name: "question1" }
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { QuestionMatrixDynamicModel, Base } from "survey-core";
import { QuestionMatrixDynamicModel, Base, QuestionTextModel } from "survey-core";
import { PropertyGridModelTester } from "./property-grid.base";
import { CreatorTester } from "../creator-tester";

test("creator.onElementDeleting", () => {
const creator = new CreatorTester();
creator.onElementDeleting.add((sender, options) => {
if(options.element.isPage) {
if (options.element.isPage) {
options.allowing = creator.survey.pages.indexOf(options.element) > 1;
}
});
Expand All @@ -25,3 +25,77 @@ test("creator.onElementDeleting", () => {
pagesQuestion.removeRowUI(rows[2]);
expect(survey.pages).toHaveLength(2);
});

test("Test event prevent default on typing text update mode, fix undo/redo #4889", () => {
const testInput = document.createElement("input");
const fakeInput = document.createElement("input");
document.body.appendChild(testInput);
document.body.appendChild(fakeInput);

const question = new QuestionTextModel("q");
const creator = new CreatorTester();
creator.registerShortcut("undo_test", {
name: "undo",
affectedTab: "designer",
hotKey: {
ctrlKey: true,
keyCode: 90,
},
macOsHotkey: {
keyCode: 90,
},
execute: () => { log += "->undo"; }
});
creator.registerShortcut("redo_test", {
name: "redo",
affectedTab: "designer",
hotKey: {
ctrlKey: true,
keyCode: 89,
},
macOsHotkey: {
keyCode: 89,
},
execute: () => { log += "->redo"; }
});
let propertyGrid = new PropertyGridModelTester(question, creator);
let questionName_RequiredOnBlur = propertyGrid.survey.getQuestionByName("name");
let questionTitle_OnTyping = propertyGrid.survey.getQuestionByName("title");

const eventCtrlZ = {
target: testInput,
keyCode: 90,
ctrlKey: true,
preventDefault: () => { log += "->preventDefaultZ"; }
};
const eventCtrlY = {
target: testInput,
keyCode: 89,
ctrlKey: true,
preventDefault: () => { log += "->preventDefaultY"; }
};

let log = "";
questionName_RequiredOnBlur.onKeyDown(eventCtrlZ);
expect(log).toBe(""); //, "text ctrl+Z for required property - call built-in undo

questionName_RequiredOnBlur.onKeyDown(eventCtrlY);
expect(log).toBe(""); //, "text ctrl+Y" for required property - call built-in redo

testInput.value = questionName_RequiredOnBlur.value;

questionName_RequiredOnBlur.onKeyDown(eventCtrlZ);
expect(log).toBe("->undo"); //, "text ctrl+Z for required property - call creator undo, built-in undo will do nothing for the save values

questionName_RequiredOnBlur.onKeyDown(eventCtrlY);
expect(log).toBe("->undo->redo"); //, "text ctrl+Y" for required property - call creator redo, built-in undo will do nothing for the save values

questionTitle_OnTyping.onKeyDown(eventCtrlZ);
expect(log).toBe("->undo->redo->undo->preventDefaultZ"); //, "comment ctrl+Z" for onTyping property

questionTitle_OnTyping.onKeyDown(eventCtrlY);
expect(log).toBe("->undo->redo->undo->preventDefaultZ->redo->preventDefaultY"); //, "comment ctrl+Y" for onTyping property

testInput.remove();
fakeInput.remove();
});
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ test("surveypages property editor and onCollectionItemAllowingCallback", () => {
item: Base,
options: ICollectionItemAllowOperations
): void => {
if(property.name !== "pages") return;
if (property.name !== "pages") return;
const name = (<any>item).name;
options.allowDelete = name !== "page2";
options.allowEdit = name !== "page2";
Expand Down Expand Up @@ -2016,7 +2016,7 @@ test("Required properties restore on change to empty value", (): any => {
var question = new QuestionTextModel("q1");
question.title = "q1Title";
var options = new EmptySurveyCreatorOptions();
options["survey"] = { getAllQuestions: ()=> [question] };
options["survey"] = { getAllQuestions: () => [question] };
var propertyGrid = new PropertyGridModelTester(question, options);
var titleQuestion = propertyGrid.survey.getQuestionByName("title") as QuestionTextModel;
titleQuestion.value = "q1t";
Expand Down Expand Up @@ -2829,7 +2829,7 @@ test("PropertyEditor for question name", () => {
const checkedData = ["Row", "panel", "choice", "Item"];
const errorText = "Do not use reserved words: \"item\", \"choice\", \"panel\", \"row\".";
let prevName = question.name;
for(let i = 0; i < checkedData.length; i ++) {
for (let i = 0; i < checkedData.length; i++) {
const erroredName = checkedData[i];
const validName = "q" + (i + 1).toString();
nameQuestion.value = erroredName;
Expand All @@ -2845,7 +2845,7 @@ test("PropertyEditor for question name", () => {
const panel = new PanelModel("p");
propertyGrid = new PropertyGridModelTester(panel);
nameQuestion = propertyGrid.survey.getQuestionByName("name");
for(let i = 0; i < checkedData.length; i ++) {
for (let i = 0; i < checkedData.length; i++) {
const erroredName = checkedData[i];
nameQuestion.value = erroredName;
expect(nameQuestion.value).toBe(erroredName);
Expand Down Expand Up @@ -2879,4 +2879,4 @@ test("Allow to enter one space into question title #4416", () => {
const propertyGrid2 = new PropertyGridModelTester(question);
const titleQuestion2 = <QuestionMatrixDynamicModel>(propertyGrid2.survey.getQuestionByName("title"));
expect(titleQuestion2.value).toBe(" ");
});
});

0 comments on commit a242b63

Please sign in to comment.