fix: address additional PR review issues
This commit is contained in:
@@ -999,7 +999,8 @@ export class UIManager {
|
||||
const widgetType = row.dataset.widgetType || 'text';
|
||||
const rawValue = row.dataset.rawValue;
|
||||
|
||||
if (!nodeId || !widgetName) return;
|
||||
// Use isNaN check instead of !nodeId to allow nodeId 0
|
||||
if (isNaN(nodeId) || !widgetName) return;
|
||||
|
||||
// Get current constraints from the data extractor
|
||||
let constraints: any = {};
|
||||
@@ -1014,6 +1015,7 @@ export class UIManager {
|
||||
}
|
||||
|
||||
// Create the editor
|
||||
const editorKey = `${nodeId}:${widgetName}`;
|
||||
const editor = WidgetEditorFactory.createEditor({
|
||||
nodeId,
|
||||
widgetName,
|
||||
@@ -1026,16 +1028,20 @@ export class UIManager {
|
||||
},
|
||||
onBlur: () => {
|
||||
// Exit edit mode after a small delay (allows for clicks within editor)
|
||||
// Capture the current editor reference to avoid race condition
|
||||
const currentEditor = this.activeEditors.get(editorKey);
|
||||
setTimeout(() => {
|
||||
if (!container.contains(document.activeElement)) {
|
||||
this.exitEditMode(row, valueEl, container);
|
||||
// Only exit if the same editor is still active (not replaced by a new one)
|
||||
if (currentEditor && this.activeEditors.get(editorKey) === currentEditor) {
|
||||
if (!container.contains(document.activeElement)) {
|
||||
this.exitEditMode(row, valueEl, container);
|
||||
}
|
||||
}
|
||||
}, 100);
|
||||
}
|
||||
});
|
||||
|
||||
// Track the editor
|
||||
const editorKey = `${nodeId}:${widgetName}`;
|
||||
this.activeEditors.set(editorKey, editor);
|
||||
|
||||
// Show editor, hide value
|
||||
|
||||
@@ -122,7 +122,12 @@ export class WidgetEditorFactory {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
const step = config.constraints?.step ?? 1;
|
||||
input.value = String(parseFloat(input.value) - step);
|
||||
let newValue = parseFloat(input.value) - step;
|
||||
// Clamp to constraints
|
||||
if (config.constraints?.min !== undefined) {
|
||||
newValue = Math.max(config.constraints.min, newValue);
|
||||
}
|
||||
input.value = String(newValue);
|
||||
syncValue();
|
||||
});
|
||||
|
||||
@@ -130,7 +135,12 @@ export class WidgetEditorFactory {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
const step = config.constraints?.step ?? 1;
|
||||
input.value = String(parseFloat(input.value) + step);
|
||||
let newValue = parseFloat(input.value) + step;
|
||||
// Clamp to constraints
|
||||
if (config.constraints?.max !== undefined) {
|
||||
newValue = Math.min(config.constraints.max, newValue);
|
||||
}
|
||||
input.value = String(newValue);
|
||||
syncValue();
|
||||
});
|
||||
|
||||
@@ -243,11 +253,14 @@ export class WidgetEditorFactory {
|
||||
|
||||
// Populate options
|
||||
const options = config.constraints?.options ?? [];
|
||||
const currentValueStr = String(config.currentValue);
|
||||
for (const opt of options) {
|
||||
const option = document.createElement('option');
|
||||
option.value = String(opt);
|
||||
option.textContent = String(opt);
|
||||
if (opt === config.currentValue) {
|
||||
const optStr = String(opt);
|
||||
option.value = optStr;
|
||||
option.textContent = optStr;
|
||||
// Use string comparison to handle non-string option types
|
||||
if (optStr === currentValueStr) {
|
||||
option.selected = true;
|
||||
}
|
||||
select.appendChild(option);
|
||||
|
||||
@@ -227,10 +227,18 @@ export class WidgetSyncManager {
|
||||
|
||||
// Handle combo (validate against options)
|
||||
if (widgetType === 'combo' && constraints.options) {
|
||||
if (!constraints.options.includes(value)) {
|
||||
// Use string comparison since HTML select values are always strings
|
||||
const valueStr = String(value);
|
||||
const hasMatch = constraints.options.some(opt => String(opt) === valueStr);
|
||||
if (!hasMatch) {
|
||||
Logger.warn(`[WidgetSync] Invalid combo value "${value}", keeping current`);
|
||||
return widget.value;
|
||||
}
|
||||
// Return the original option type (may be number)
|
||||
const matchedOption = constraints.options.find(opt => String(opt) === valueStr);
|
||||
if (matchedOption !== undefined) {
|
||||
return matchedOption;
|
||||
}
|
||||
}
|
||||
|
||||
// Handle boolean
|
||||
@@ -339,9 +347,13 @@ export class WidgetSyncManager {
|
||||
}
|
||||
}
|
||||
|
||||
// Combo validation
|
||||
if (constraints.options && !constraints.options.includes(value)) {
|
||||
return { valid: false, error: `Invalid option selected` };
|
||||
// Combo validation - use string comparison
|
||||
if (constraints.options) {
|
||||
const valueStr = String(value);
|
||||
const hasMatch = constraints.options.some(opt => String(opt) === valueStr);
|
||||
if (!hasMatch) {
|
||||
return { valid: false, error: `Invalid option selected` };
|
||||
}
|
||||
}
|
||||
|
||||
return { valid: true };
|
||||
|
||||
@@ -724,7 +724,7 @@ class UIManager {
|
||||
const widgetName = row.dataset.widgetName || "";
|
||||
const widgetType = row.dataset.widgetType || "text";
|
||||
const rawValue = row.dataset.rawValue;
|
||||
if (!nodeId || !widgetName) return;
|
||||
if (isNaN(nodeId) || !widgetName) return;
|
||||
let constraints = {};
|
||||
try {
|
||||
const constraintsStr = row.dataset.constraints;
|
||||
@@ -733,6 +733,7 @@ class UIManager {
|
||||
}
|
||||
} catch (e) {
|
||||
}
|
||||
const editorKey = `${nodeId}:${widgetName}`;
|
||||
const editor = WidgetEditorFactory.createEditor({
|
||||
nodeId,
|
||||
widgetName,
|
||||
@@ -743,14 +744,16 @@ class UIManager {
|
||||
row.dataset.rawValue = String(value);
|
||||
},
|
||||
onBlur: () => {
|
||||
const currentEditor = this.activeEditors.get(editorKey);
|
||||
setTimeout(() => {
|
||||
if (!container.contains(document.activeElement)) {
|
||||
this.exitEditMode(row, valueEl, container);
|
||||
if (currentEditor && this.activeEditors.get(editorKey) === currentEditor) {
|
||||
if (!container.contains(document.activeElement)) {
|
||||
this.exitEditMode(row, valueEl, container);
|
||||
}
|
||||
}
|
||||
}, 100);
|
||||
}
|
||||
});
|
||||
const editorKey = `${nodeId}:${widgetName}`;
|
||||
this.activeEditors.set(editorKey, editor);
|
||||
row.classList.add("editing");
|
||||
valueEl.style.display = "none";
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -72,19 +72,27 @@ class WidgetEditorFactory {
|
||||
return (_a = config.onBlur) == null ? void 0 : _a.call(config);
|
||||
});
|
||||
decrementBtn.addEventListener("click", (e) => {
|
||||
var _a;
|
||||
var _a, _b;
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
const step = ((_a = config.constraints) == null ? void 0 : _a.step) ?? 1;
|
||||
input.value = String(parseFloat(input.value) - step);
|
||||
let newValue = parseFloat(input.value) - step;
|
||||
if (((_b = config.constraints) == null ? void 0 : _b.min) !== void 0) {
|
||||
newValue = Math.max(config.constraints.min, newValue);
|
||||
}
|
||||
input.value = String(newValue);
|
||||
syncValue();
|
||||
});
|
||||
incrementBtn.addEventListener("click", (e) => {
|
||||
var _a;
|
||||
var _a, _b;
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
const step = ((_a = config.constraints) == null ? void 0 : _a.step) ?? 1;
|
||||
input.value = String(parseFloat(input.value) + step);
|
||||
let newValue = parseFloat(input.value) + step;
|
||||
if (((_b = config.constraints) == null ? void 0 : _b.max) !== void 0) {
|
||||
newValue = Math.min(config.constraints.max, newValue);
|
||||
}
|
||||
input.value = String(newValue);
|
||||
syncValue();
|
||||
});
|
||||
input.addEventListener("keydown", (e) => {
|
||||
@@ -194,11 +202,13 @@ class WidgetEditorFactory {
|
||||
const select = document.createElement("select");
|
||||
select.className = "widget-editor-select";
|
||||
const options = ((_a = config.constraints) == null ? void 0 : _a.options) ?? [];
|
||||
const currentValueStr = String(config.currentValue);
|
||||
for (const opt of options) {
|
||||
const option = document.createElement("option");
|
||||
option.value = String(opt);
|
||||
option.textContent = String(opt);
|
||||
if (opt === config.currentValue) {
|
||||
const optStr = String(opt);
|
||||
option.value = optStr;
|
||||
option.textContent = optStr;
|
||||
if (optStr === currentValueStr) {
|
||||
option.selected = true;
|
||||
}
|
||||
select.appendChild(option);
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -139,10 +139,16 @@ class WidgetSyncManager {
|
||||
return numValue;
|
||||
}
|
||||
if (widgetType === "combo" && constraints.options) {
|
||||
if (!constraints.options.includes(value)) {
|
||||
const valueStr = String(value);
|
||||
const hasMatch = constraints.options.some((opt) => String(opt) === valueStr);
|
||||
if (!hasMatch) {
|
||||
Logger.warn(`[WidgetSync] Invalid combo value "${value}", keeping current`);
|
||||
return widget.value;
|
||||
}
|
||||
const matchedOption = constraints.options.find((opt) => String(opt) === valueStr);
|
||||
if (matchedOption !== void 0) {
|
||||
return matchedOption;
|
||||
}
|
||||
}
|
||||
if (widgetType === "toggle" || widgetType === "boolean") {
|
||||
return Boolean(value);
|
||||
@@ -223,8 +229,12 @@ class WidgetSyncManager {
|
||||
return { valid: false, error: `Value must be at most ${constraints.max}` };
|
||||
}
|
||||
}
|
||||
if (constraints.options && !constraints.options.includes(value)) {
|
||||
return { valid: false, error: `Invalid option selected` };
|
||||
if (constraints.options) {
|
||||
const valueStr = String(value);
|
||||
const hasMatch = constraints.options.some((opt) => String(opt) === valueStr);
|
||||
if (!hasMatch) {
|
||||
return { valid: false, error: `Invalid option selected` };
|
||||
}
|
||||
}
|
||||
return { valid: true };
|
||||
}
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user