Skip to content

Commit 12ac358

Browse files
authored
fix: loading tooltips before messages (#7504)
* fix: loading tooltips before messages * fix: typing
1 parent 3b234c7 commit 12ac358

1 file changed

Lines changed: 20 additions & 81 deletions

File tree

core/extensions.ts

Lines changed: 20 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -388,16 +388,6 @@ export function runAfterPageLoad(fn: () => void) {
388388
* Builds an extension function that will map a dropdown value to a tooltip
389389
* string.
390390
*
391-
* This method includes multiple checks to ensure tooltips, dropdown options,
392-
* and message references are aligned. This aims to catch errors as early as
393-
* possible, without requiring developers to manually test tooltips under each
394-
* option. After the page is loaded, each tooltip text string will be checked
395-
* for matching message keys in the internationalized string table. Deferring
396-
* this until the page is loaded decouples loading dependencies. Later, upon
397-
* loading the first block of any given type, the extension will validate every
398-
* dropdown option has a matching tooltip in the lookupTable. Errors are
399-
* reported as warnings in the console, and are never fatal.
400-
*
401391
* @param dropdownName The name of the field whose value is the key to the
402392
* lookup table.
403393
* @param lookupTable The table of field values to tooltip text.
@@ -406,56 +396,23 @@ export function runAfterPageLoad(fn: () => void) {
406396
export function buildTooltipForDropdown(
407397
dropdownName: string,
408398
lookupTable: {[key: string]: string},
409-
): Function {
399+
): (this: Block) => void {
410400
// List of block types already validated, to minimize duplicate warnings.
411-
const blockTypesChecked: AnyDuringMigration[] = [];
412-
413-
// Check the tooltip string messages for invalid references.
414-
// Wait for load, in case Blockly.Msg is not yet populated.
415-
// runAfterPageLoad() does not run in a Node.js environment due to lack
416-
// of document object, in which case skip the validation.
417-
if (typeof document === 'object') {
418-
// Relies on document.readyState
419-
runAfterPageLoad(function () {
420-
for (const key in lookupTable) {
421-
// Will print warnings if reference is missing.
422-
parsing.checkMessageReferences(lookupTable[key]);
423-
}
424-
});
425-
}
401+
const blockTypesChecked: string[] = [];
426402

427-
/** The actual extension. */
428-
function extensionFn(this: Block) {
429-
if (this.type && blockTypesChecked.indexOf(this.type) === -1) {
403+
return function (this: Block) {
404+
if (blockTypesChecked.indexOf(this.type) === -1) {
430405
checkDropdownOptionsInTable(this, dropdownName, lookupTable);
431406
blockTypesChecked.push(this.type);
432407
}
433408

434409
this.setTooltip(
435410
function (this: Block) {
436411
const value = String(this.getFieldValue(dropdownName));
437-
let tooltip = lookupTable[value];
438-
if (tooltip === null) {
439-
if (blockTypesChecked.indexOf(this.type) === -1) {
440-
// Warn for missing values on generated tooltips.
441-
let warning =
442-
'No tooltip mapping for value ' +
443-
value +
444-
' of field ' +
445-
dropdownName;
446-
if (this.type !== null) {
447-
warning += ' of block type ' + this.type;
448-
}
449-
console.warn(warning + '.');
450-
}
451-
} else {
452-
tooltip = parsing.replaceMessageReferences(tooltip);
453-
}
454-
return tooltip;
412+
return parsing.replaceMessageReferences(lookupTable[value]);
455413
}.bind(this),
456414
);
457-
}
458-
return extensionFn;
415+
};
459416
}
460417

461418
/**
@@ -471,22 +428,18 @@ function checkDropdownOptionsInTable(
471428
dropdownName: string,
472429
lookupTable: {[key: string]: string},
473430
) {
474-
// Validate all dropdown options have values.
475431
const dropdown = block.getField(dropdownName);
476-
if (dropdown instanceof FieldDropdown && !dropdown.isOptionListDynamic()) {
477-
const options = dropdown.getOptions();
478-
for (let i = 0; i < options.length; i++) {
479-
const optionKey = options[i][1]; // label, then value
480-
if (lookupTable[optionKey] === null) {
481-
console.warn(
482-
'No tooltip mapping for value ' +
483-
optionKey +
484-
' of field ' +
485-
dropdownName +
486-
' of block type ' +
487-
block.type,
488-
);
489-
}
432+
if (!(dropdown instanceof FieldDropdown) || dropdown.isOptionListDynamic()) {
433+
return;
434+
}
435+
436+
const options = dropdown.getOptions();
437+
for (const [, key] of options) {
438+
if (lookupTable[key] === undefined) {
439+
console.warn(
440+
`No tooltip mapping for value ${key} of field ` +
441+
`${dropdownName} of block type ${block.type}.`,
442+
);
490443
}
491444
}
492445
}
@@ -504,21 +457,8 @@ function checkDropdownOptionsInTable(
504457
export function buildTooltipWithFieldText(
505458
msgTemplate: string,
506459
fieldName: string,
507-
): Function {
508-
// Check the tooltip string messages for invalid references.
509-
// Wait for load, in case Blockly.Msg is not yet populated.
510-
// runAfterPageLoad() does not run in a Node.js environment due to lack
511-
// of document object, in which case skip the validation.
512-
if (typeof document === 'object') {
513-
// Relies on document.readyState
514-
runAfterPageLoad(function () {
515-
// Will print warnings if reference is missing.
516-
parsing.checkMessageReferences(msgTemplate);
517-
});
518-
}
519-
520-
/** The actual extension. */
521-
function extensionFn(this: Block) {
460+
): (this: Block) => void {
461+
return function (this: Block) {
522462
this.setTooltip(
523463
function (this: Block) {
524464
const field = this.getField(fieldName);
@@ -527,8 +467,7 @@ export function buildTooltipWithFieldText(
527467
.replace('%1', field ? field.getText() : '');
528468
}.bind(this),
529469
);
530-
}
531-
return extensionFn;
470+
};
532471
}
533472

534473
/**

0 commit comments

Comments
 (0)