Skip to content

Commit b2c5e49

Browse files
committed
Fix review issues: translations, memory leak, destroy, storage key, ARIA, ID entropy
- Add 'View' and 'Table Columns' to en/commerce.php and InventoryAsset.php (remove unused 'View settings') - rebuildTable: destroy old VueAdminTable before emptying DOM to prevent instance leak - rebuildTable: add 50-attempt escape hatch to setInterval search restore - Add destroy() override to clean up toolbar DOM, viewMenu DOM, and DisclosureMenu instance - getStorageKey: use inventoryLocationId (per-location prefs) instead of siteUid - Remove mismatched aria-label from View button (visible text is sufficient) - Improve random ID entropy: substring(2) instead of substring(7)
1 parent 388b3db commit b2c5e49

5 files changed

Lines changed: 36 additions & 13 deletions

File tree

src/translations/en/commerce.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,7 @@
11011101
'Switch plan' => 'Switch plan',
11021102
'Switch' => 'Switch',
11031103
'System' => 'System',
1104+
'Table Columns' => 'Table Columns',
11041105
'Target - The category relationship field is on the purchasable' => 'Target - The category relationship field is on the purchasable',
11051106
'Tax & Shipping' => 'Tax & Shipping',
11061107
'Tax (inc)' => 'Tax (inc)',
@@ -1300,6 +1301,7 @@
13001301
'Variants not restored.' => 'Variants not restored.',
13011302
'Variants restored.' => 'Variants restored.',
13021303
'Variants' => 'Variants',
1304+
'View' => 'View',
13031305
'View customer' => 'View customer',
13041306
'View order' => 'View order',
13051307
'View product type - {productType}' => 'View product type - {productType}',

src/web/assets/inventory/InventoryAsset.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ public function registerAssetFiles($view): void
6363
'On Hand',
6464
'Incoming',
6565
'View',
66-
'View settings',
6766
'Table Columns',
6867
'Purchasable',
6968
'SKU',

src/web/assets/inventory/dist/inventory.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/web/assets/inventory/dist/inventory.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/web/assets/inventory/src/js/InventoryLevelsManager.js

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ Craft.Commerce.InventoryLevelsManager = Garnish.Base.extend({
4949

5050
// random id for the admin table
5151
this.adminTableId =
52-
'inventory-admin-table-' + Math.random().toString(36).substring(7);
52+
'inventory-admin-table-' + Math.random().toString(36).substring(2);
5353
this.$adminTable = $('<div id="' + this.adminTableId + '"></div>').appendTo(
5454
this.$container
5555
);
@@ -59,9 +59,9 @@ Craft.Commerce.InventoryLevelsManager = Garnish.Base.extend({
5959

6060
getStorageKey: function () {
6161
return (
62-
'Craft-' +
63-
Craft.siteUid +
64-
'.Commerce.InventoryLevels.viewPreferences.' +
62+
'Commerce.InventoryLevels.viewPreferences.' +
63+
this.settings.inventoryLocationId +
64+
'.' +
6565
Craft.userId
6666
);
6767
},
@@ -212,7 +212,7 @@ Craft.Commerce.InventoryLevelsManager = Garnish.Base.extend({
212212

213213
// Generate unique ID for the menu
214214
this.viewMenuId =
215-
'inventory-view-menu-' + Math.random().toString(36).substring(7);
215+
'inventory-view-menu-' + Math.random().toString(36).substring(2);
216216

217217
// Create a toolbar container for the view button (positioned at top right)
218218
this.$viewToolbar = $('<div/>', {
@@ -227,7 +227,6 @@ Craft.Commerce.InventoryLevelsManager = Garnish.Base.extend({
227227
type: 'button',
228228
class: 'btn menubtn',
229229
text: Craft.t('commerce', 'View'),
230-
'aria-label': Craft.t('commerce', 'View settings'),
231230
'aria-controls': this.viewMenuId,
232231
'data-icon': 'sliders',
233232
}).appendTo(this.$viewToolbar);
@@ -352,27 +351,50 @@ Craft.Commerce.InventoryLevelsManager = Garnish.Base.extend({
352351
var searchValue =
353352
this.$container.find('.vue-admin-table input[type="search"]').val() || '';
354353

355-
// Remove the old table
354+
// Destroy the old Vue instance before removing its DOM
355+
if (this.adminTable && typeof this.adminTable.destroy === 'function') {
356+
this.adminTable.destroy();
357+
}
358+
356359
this.$adminTable.empty();
357360

358361
// Reinitialize with new columns
359362
this.initAdminTable();
360363

361364
// Reapply search if there was one
362365
if (searchValue) {
366+
var attempts = 0;
363367
var checkSearch = setInterval(() => {
364368
var $searchInput = this.$container.find(
365369
'.vue-admin-table input[type="search"]'
366370
);
367-
if ($searchInput.length) {
371+
if ($searchInput.length || ++attempts > 50) {
368372
clearInterval(checkSearch);
369-
$searchInput.val(searchValue);
370-
$searchInput.trigger('input');
373+
if ($searchInput.length) {
374+
$searchInput.val(searchValue);
375+
$searchInput.trigger('input');
376+
}
371377
}
372378
}, 100);
373379
}
374380
},
375381

382+
destroy: function () {
383+
if (this.$viewToolbar) {
384+
this.$viewToolbar.remove();
385+
}
386+
if (this.$viewMenu) {
387+
this.$viewMenu.remove();
388+
}
389+
if (this.viewMenu && typeof this.viewMenu.destroy === 'function') {
390+
this.viewMenu.destroy();
391+
}
392+
if (this.adminTable && typeof this.adminTable.destroy === 'function') {
393+
this.adminTable.destroy();
394+
}
395+
this.base();
396+
},
397+
376398
defaultSettings: {
377399
inventoryLocationId: null,
378400
inventoryItemId: null,

0 commit comments

Comments
 (0)