Bug/6587 focus on error (#6611)

* When the focusOnFirstError option is enabled, a survey focuses the first input on a page instead of the first invalid question fix #6587

* Remove unneeded wait in functional test #6587

* Fix functional tests #6587

* Update etalons for visual tests with focusing #6587

* Improve notofier window visual tests #6587

src/base-interfaces.ts

@@ -45,6 +45,7 @@ export interface ISurveyErrorOwner extends ILocalizableOwner {
 export interface ISurvey extends ITextProcessor, ISurveyErrorOwner {
   getSkeletonComponentName(element: ISurveyElement): string;
   currentPage: IPage;
+  activePage: IPage;
   pages: Array<IPage>;
   getCss(): any;
   isPageStarted(page: IPage): boolean;
@@ -77,6 +78,7 @@ export interface ISurvey extends ITextProcessor, ISurveyErrorOwner {
     oldName: string,
     oldValueName: string
   ): any;
+  focusQuestionByInstance(question: IQuestion, onError: boolean): boolean;
   validateQuestion(question: IQuestion): SurveyError;
   validatePanel(panel: IPanel): SurveyError;
   hasVisibleQuestionByValueName(valueName: string): boolean;

src/panel.ts

@@ -524,6 +524,16 @@ export class PanelModelBase extends SurveyElement<Question>
 
     return this.questionsValue;
   }
+  public getQuestions(includeNested: boolean): Array<Question> {
+    const res = this.questions;
+    if(!includeNested) return res;
+    const res2: Array<Question> = [];
+    res.forEach(q => {
+      res2.push(q);
+      q.getNestedQuestions().forEach(nQ => res2.push(nQ));
+    });
+    return res2;
+  }
   protected getValidName(name: string): string {
     if (!!name) return name.trim();
     return name;

src/question.ts

@@ -249,6 +249,7 @@ export class Question extends SurveyElement<Question>
    * Returns a page to which the question belongs and allows you to move this question to a different page.
    */
   public get page(): IPage {
+    if(!!this.parentQuestion) return this.parentQuestion.page;
     return this.getPage(this.parent);
   }
   public set page(val: IPage) {
@@ -969,13 +970,9 @@ export class Question extends SurveyElement<Question>
   public focus(onError: boolean = false): void {
     if (this.isDesignMode || !this.isVisible || !this.survey) return;
     let page = this.page;
-    if(!page && !!this.parentQuestion) {
-      page = this.parentQuestion.page;
-    }
-    let shouldChangePage = !!page && this.survey.currentPage !== page;
+    const shouldChangePage = !!page && this.survey.activePage !== page;
     if(shouldChangePage) {
-      this.survey.currentPage = page;
-      setTimeout(() => this.focuscore(onError), 0);
+      this.survey.focusQuestionByInstance(this, onError);
     } else {
       this.focuscore(onError);
     }

src/survey-element.ts

@@ -201,7 +201,7 @@ export class SurveyElement<E = any> extends SurveyElementCore implements ISurvey
     const { root } = settings.environment;
     if (!root) return false;
     const el = root.getElementById(elementId);
-    if (el && !(<any>el)["disabled"]) {
+    if (el && !(<any>el)["disabled"] && el.style.display !== "none") {
       el.focus();
       return true;
     }

src/survey.ts

@@ -3081,7 +3081,7 @@ export class SurveyModel extends SurveyElementCore
    * @see focusFirstQuestionAutomatic
    */
   public focusFirstQuestion() {
-    if (this.isFocusingQuestion) return;
+    if (this.focusingQuestionInfo) return;
     var page = this.activePage;
     if (page) {
       page.scrollToTop();
@@ -3094,7 +3094,7 @@ export class SurveyModel extends SurveyElementCore
     if (doScroll) {
       page.scrollToTop();
     }
-    if (this.isCurrentPageRendering && this.focusFirstQuestionAutomatic && !this.isFocusingQuestion) {
+    if (this.isCurrentPageRendering && this.focusFirstQuestionAutomatic && !this.focusingQuestionInfo) {
       page.focusFirstQuestion();
       this.isCurrentPageRendering = false;
     }
@@ -3614,9 +3614,8 @@ export class SurveyModel extends SurveyElementCore
       }
     }
     if (focusOnFirstError && !!firstErrorPage) {
-      this.currentPage = firstErrorPage;
-      var questions = firstErrorPage.questions;
-      for (var i = 0; i < questions.length; i++) {
+      const questions = firstErrorPage.getQuestions(true);
+      for (let i = 0; i < questions.length; i++) {
         if (questions[i].errors.length > 0) {
           questions[i].focus(true);
           break;
@@ -4216,6 +4215,7 @@ export class SurveyModel extends SurveyElementCore
    */
   public start(): boolean {
     if (!this.firstPageIsStarted) return false;
+    this.isCurrentPageRendering = true;
     if (this.checkIsPageHasErrors(this.startedPage, true)) return false;
     this.isStartedState = false;
     this.startTimerFromUI();
@@ -4550,12 +4550,10 @@ export class SurveyModel extends SurveyElementCore
   private isFirstPageRendering: boolean = true;
   private isCurrentPageRendering: boolean = true;
   afterRenderPage(htmlElement: HTMLElement) {
-    if (!this.isDesignMode && !this.isFocusingQuestion) {
+    if (!this.isDesignMode && !this.focusingQuestionInfo) {
       setTimeout(() => this.scrollToTopOnPageChange(!this.isFirstPageRendering), 1);
     }
-    while (this.afterRenderPageTasks.length > 0) {
-      this.afterRenderPageTasks.shift()();
-    }
+    this.focusQuestionInfo();
     this.isFirstPageRendering = false;
     if (this.onAfterRenderPage.isEmpty) return;
     this.onAfterRenderPage.fire(this, {
@@ -6958,8 +6956,7 @@ export class SurveyModel extends SurveyElementCore
   triggerExecuted(trigger: Trigger): void {
     this.onTriggerExecuted.fire(this, { trigger: trigger });
   }
-  private isFocusingQuestion: boolean;
-  private afterRenderPageTasks: Array<() => void> = [];
+  private focusingQuestionInfo: any;
   private isMovingQuestion: boolean;
   public startMovingQuestion(): void {
     this.isMovingQuestion = true;
@@ -6979,24 +6976,30 @@ export class SurveyModel extends SurveyElementCore
    * @see focusFirstQuestionAutomatic
    */
   public focusQuestion(name: string): boolean {
-    var question = this.getQuestionByName(name, true);
+    return this.focusQuestionByInstance(this.getQuestionByName(name, true));
+  }
+  focusQuestionByInstance(question: Question, onError: boolean = false): boolean {
     if (!question || !question.isVisible || !question.page) return false;
-    this.isFocusingQuestion = true;
+    const oldQuestion = this.focusingQuestionInfo?.question;
+    if(oldQuestion === question) return false;
+    this.focusingQuestionInfo = { question: question, onError: onError };
     this.skippedPages.push({ from: this.currentPage, to: question.page });
-    const isNeedWaitForPageRendered = this.currentPage !== question.page;
-    const focusQuestionFunc = () => {
-      question.focus();
-      this.isFocusingQuestion = false;
-      this.isCurrentPageRendering = false;
-    };
-    this.afterRenderPageTasks.push(focusQuestionFunc);
-    this.currentPage = <PageModel>question.page;
+    const isNeedWaitForPageRendered = this.activePage !== question.page && !question.page.isStartPage;
+    if(isNeedWaitForPageRendered) {
+      this.currentPage = <PageModel>question.page;
+    }
     if (!isNeedWaitForPageRendered) {
-      focusQuestionFunc();
-      this.afterRenderPageTasks.splice(this.afterRenderPageTasks.indexOf(focusQuestionFunc), 1);
+      this.focusQuestionInfo();
     }
     return true;
   }
+  private focusQuestionInfo(): void {
+    const question = this.focusingQuestionInfo?.question;
+    if(!!question && !question.isDisposed) {
+      question.focus(this.focusingQuestionInfo.onError);
+    }
+    this.focusingQuestionInfo = undefined;
+  }
 
   public questionEditFinishCallback(question: Question, event: any) {
     const enterKeyAction = this.enterKeyAction || settings.enterKeyAction;

testCafe/validation/focusErroredInput.js

@@ -0,0 +1,125 @@
+import { frameworks, url, initSurvey, getSurveyResult } from "../helper";
+import { Selector, ClientFunction, fixture, test } from "testcafe";
+const title = "focus input with Error";
+const json1 = {
+  "pages": [
+    {
+      "name": "page1",
+      "elements": [
+        {
+          "type": "text",
+          "name": "question1"
+        },
+        {
+          "type": "boolean",
+          "name": "question2"
+        },
+
+        {
+          "type": "text",
+          "name": "q1",
+          "validators": [{ "type": "numeric", "text": "Enter only numbers" }]
+        }
+      ]
+    },
+    {
+      "name": "page2",
+      "elements": [
+        {
+          "type": "text",
+          "name": "question3"
+        }
+      ]
+    }
+  ],
+  "checkErrorsMode": "onComplete"
+};
+const json2 = {
+  "pages": [
+    {
+      "name": "page1",
+      "elements": [
+        {
+          "type": "text",
+          "name": "question1"
+        },
+        {
+          "type": "boolean",
+          "name": "question2"
+        },
+
+        {
+          "type": "matrixdynamic",
+          "name": "matrix",
+          "rowCount": 1,
+          "columns": [
+            { "cellType": "text", "name": "col1",
+              "validators": [{ "type": "numeric", "text": "Enter only numbers" }] }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "page2",
+      "elements": [
+        {
+          "type": "text",
+          "name": "question3"
+        }
+      ]
+    }
+  ],
+  "checkErrorsMode": "onComplete"
+};
+
+frameworks.forEach(framework => {
+  fixture`${framework} ${title}`.page`${url}${framework}`.beforeEach(
+    async t => {
+      await initSurvey(framework, json1);
+    }
+  );
+
+  test("validate on error", async t => {
+    let surveyResult;
+
+    await t
+      .pressKey("tab")
+      .pressKey("tab")
+      .pressKey("a")
+      .click("input[value=\"Next\"]")
+      .click("input[value=\"Complete\"]")
+      .pressKey("backspace")
+      .pressKey("1")
+      .click("input[value=\"Next\"]")
+      .click("input[value=\"Complete\"]");
+
+    surveyResult = await getSurveyResult();
+    await t.expect(surveyResult).eql({ q1: "1" });
+  });
+});
+
+frameworks.forEach(framework => {
+  fixture`${framework} ${title}`.page`${url}${framework}`.beforeEach(
+    async t => {
+      await initSurvey(framework, json2);
+    }
+  );
+
+  test("validate on error in matrix", async t => {
+    let surveyResult;
+
+    await t
+      .pressKey("tab")
+      .pressKey("tab")
+      .pressKey("a")
+      .click("input[value=\"Next\"]")
+      .click("input[value=\"Complete\"]")
+      .pressKey("backspace")
+      .pressKey("1")
+      .click("input[value=\"Next\"]")
+      .click("input[value=\"Complete\"]");
+
+    surveyResult = await getSurveyResult();
+    await t.expect(surveyResult).eql({ matrix: [{ col1: "1" }] });
+  });
+});

tests/surveytests.ts

@@ -1469,6 +1469,7 @@ QUnit.test("survey.checkErrorsMode = 'onComplete'", function (assert) {
   assert.equal(survey.currentPageNo, 1, "Ignore error on the first page");
   survey.completeLastPage();
   assert.equal(survey.currentPageNo, 0, "Move to first page with the error");
+  survey.afterRenderPage(<HTMLElement>{});
 
   survey.nextPage();
   assert.equal(survey.currentPageNo, 1, "Ignore error on the first page, #2");
@@ -1478,6 +1479,7 @@ QUnit.test("survey.checkErrorsMode = 'onComplete'", function (assert) {
     0,
     "Move to first page with the error, #2"
   );
+  survey.afterRenderPage(<HTMLElement>{});
 
   survey.setValue("q1", "john.snow@nightwatch.org");
   survey.nextPage();
@@ -13658,6 +13660,7 @@ QUnit.test(
     survey.nextPage();
     survey.nextPage();
     survey.completeLastPage();
+    survey.afterRenderPage(<HTMLElement>{});
     assert.equal(survey.currentPageNo, 0, "The first page is active");
     assert.equal(
       survey.getQuestionByName("q1").inputId,

visualRegressionTests/tests/defaultV2/survey.ts

@@ -687,20 +687,22 @@ frameworks.forEach(framework => {
 
   test("Check survey notifier error type", async (t) => {
     await wrapVisualTest(t, async (t, comparer) => {
+      await ClientFunction(() => { (<any>window).Survey.settings.notifications.lifetime = 10000; })();
       await t.resizeWindow(1920, 900);
       await initSurvey(framework, notifierJson, { onComplete: (_sender, options) => {
         options.isCompleteOnTrigger = false;
         options.showDataSaving();
         let fail = true;
 
-        new Promise((resolve, reject) => { setTimeout(fail ? reject : resolve, 5000); }).then(
+        new Promise((resolve, reject) => { setTimeout(fail ? reject : resolve, 500); }).then(
           () => { options.showDataSavingSuccess(); },
           () => { options.showDataSavingError(); }
         );
       } });
       await setData({ nps_score: 4 });
       await t.click("input[value=\"Complete\"]");
       await takeElementScreenshot("save-data-error.png", Selector(".sv-save-data_root.sv-save-data_error"), t, comparer);
+      await ClientFunction(() => { (<any>window).Survey.settings.notifications.lifetime = 2000; })();
     });
   });
 
@@ -713,7 +715,7 @@ frameworks.forEach(framework => {
         options.showDataSaving();
         let fail = false;
 
-        new Promise((resolve, reject) => { setTimeout(fail ? reject : resolve, 5000); }).then(
+        new Promise((resolve, reject) => { setTimeout(fail ? reject : resolve, 500); }).then(
           () => { options.showDataSavingSuccess(); },
           () => { options.showDataSavingError(); }
         );