Do sorting before setting actions property (#9055)

surveyjs · Nov 16, 2024 · f4e6ef4 · f4e6ef4
1 parent 17fd13c
commit f4e6ef4
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 14 deletions.
diff --git a/packages/survey-core/src/actions/container.ts b/packages/survey-core/src/actions/container.ts
@@ -129,23 +129,31 @@ export class ActionContainer<T extends BaseAction = Action> extends Base impleme
   }
   public addAction(val: IAction, sortByVisibleIndex = true): T {
     const res: T = this.createAction(val);
-    this.actions.push(<T>res);
-    this.sortItems();
+    if(sortByVisibleIndex && !this.isActionVisible(res)) return res;
+    const items = [].concat(this.actions, res);
+    items.sort(this.compareByVisibleIndex);
+    this.actions = items;
     return res;
   }
-  private sortItems(): void {
-    this.actions = []
-      .concat(this.actions.filter((item) => item.visibleIndex === undefined || item.visibleIndex >= 0))
-      .sort((firstItem, secondItem) => {
-        return firstItem.visibleIndex - secondItem.visibleIndex;
-      });
-  }
-
   public setItems(items: Array<IAction>, sortByVisibleIndex = true): void {
-    this.actions = <any>items.map((item) => this.createAction(item));
+    const newActions: Array<T> = [];
+    items.forEach(item => {
+      if(!sortByVisibleIndex || this.isActionVisible(item)) {
+        newActions.push(this.createAction(item));
+      }
+    });
     if (sortByVisibleIndex) {
-      this.sortItems();
+      newActions.sort(this.compareByVisibleIndex);
     }
+    this.actions = newActions;
+  }
+  private compareByVisibleIndex(first: T, second: T): number {
+    if(first.visibleIndex === undefined) return 1;
+    if(second.visibleIndex === undefined) return -1;
+    return first.visibleIndex - second.visibleIndex;
+  }
+  private isActionVisible(item: IAction): boolean {
+    return item.visibleIndex >= 0 || item.visibleIndex === undefined;
   }
   @property({ defaultValue: 300 }) subItemsShowDelay: number;
   @property({ defaultValue: 300 }) subItemsHideDelay: number;

diff --git a/packages/survey-core/tests/components/actionbartests.ts b/packages/survey-core/tests/components/actionbartests.ts
@@ -153,6 +153,7 @@ QUnit.test("Action title", (assert) => {
   assert.equal(action1.title, "Select All", "Update action title en localization");
   assert.equal(action1.tooltip, "Preview", "take tooltip from en localization");
   survey.locale = "de";
+  assert.equal(action1.getLocale(), "de", "locale de");
   assert.equal(action1.locTitle.text, "Alles auswählen", "take text from de localization");
   assert.equal(action1.title, "Alles auswählen", "Update action title de localization");
   assert.equal(action1.tooltip, "Vorschau", "take tooltip from de localization");
@@ -167,11 +168,12 @@ QUnit.test("Action title in list model", (assert) => {
   });
   const list = new ListModel({ items: [action1], onSelectionChanged: () => { }, allowSelection: true } as any);
   const popupModel = new PopupModel("sv-list", list, { verticalPosition: "bottom", horizontalPosition: "center" });
-  survey.addNavigationItem({ id: "action1", title: "test", popupModel: popupModel });
+  survey.addNavigationItem({ id: "action2", title: "test", popupModel: popupModel });
   assert.equal(action1.locTitle.text, "Select All", "take text from en localization");
   assert.equal(action1.title, "Select All", "Update action title en localization");
   assert.equal(action1.tooltip, "Preview", "take tooltip from en localization");
   survey.locale = "de";
+  assert.equal(action1.getLocale(), "de");
   assert.equal(action1.locTitle.text, "Alles auswählen", "take text from de localization");
   assert.equal(action1.title, "Alles auswählen", "Update action title de localization");
   assert.equal(action1.tooltip, "Vorschau", "take tooltip from de localization");

diff --git a/packages/survey-core/tests/components/popuptests.ts b/packages/survey-core/tests/components/popuptests.ts
@@ -1616,7 +1616,7 @@ QUnit.test("Check that modal popup prevents scroll outside", (assert) => {
 
   const model: PopupModel = new PopupModel("sv-list", {});
   model.isModal = true;
-  const viewModel: PopupModalViewModel = createPopupViewModelTest(model) as PopupModalViewModel;
+  const viewModel: PopupModalViewModel = createPopupViewModelTest(model, undefined) as PopupModalViewModel;
 
   const container = document.createElement("div");
   container.innerHTML = `<div style="height: 200px; overflow: auto;">