diff --git a/core/block.ts b/core/block.ts index b1db8df5662..25856787440 100644 --- a/core/block.ts +++ b/core/block.ts @@ -939,6 +939,19 @@ export class Block implements IASTNodeLocation, IDeletable { return this.disposed; } + /** + * @returns True if this block is a value block with a single editable field. + * @internal + */ + isSimpleReporter(): boolean { + if (!this.outputConnection) return false; + + for (const input of this.inputList) { + if (input.connection || input.fieldRow.length > 1) return false; + } + return true; + } + /** * Find the connection on this block that corresponds to the given connection * on the other block. diff --git a/core/field.ts b/core/field.ts index dc7a638aad8..529c5aaef14 100644 --- a/core/field.ts +++ b/core/field.ts @@ -330,6 +330,18 @@ export abstract class Field */ initModel() {} + /** + * Defines whether this field should take up the full block or not. + * + * Be cautious when overriding this function. It may not work as you expect / + * intend because the behavior was kind of hacked in. If you are thinking + * about overriding this function, post on the forum with your intended + * behavior to see if there's another approach. + */ + protected isFullBlockField(): boolean { + return !this.borderRect_; + } + /** * Create a field border rect element. Not to be overridden by subclasses. * Instead modify the result of the function inside initView, or create a @@ -797,7 +809,7 @@ export abstract class Field const xOffset = margin !== undefined ? margin - : this.borderRect_ + : !this.isFullBlockField() ? this.getConstants()!.FIELD_BORDER_RECT_X_PADDING : 0; let totalWidth = xOffset * 2; @@ -813,7 +825,7 @@ export abstract class Field ); totalWidth += contentWidth; } - if (this.borderRect_) { + if (!this.isFullBlockField()) { totalHeight = Math.max(totalHeight, constants!.FIELD_BORDER_RECT_HEIGHT); } @@ -922,7 +934,7 @@ export abstract class Field throw new UnattachedFieldError(); } - if (!this.borderRect_) { + if (this.isFullBlockField()) { // Browsers are inconsistent in what they return for a bounding box. // - Webkit / Blink: fill-box / object bounding box // - Gecko: stroke-box @@ -940,8 +952,8 @@ export abstract class Field xy.y -= 0.5 * scale; } } else { - const bBox = this.borderRect_.getBoundingClientRect(); - xy = style.getPageOffset(this.borderRect_); + const bBox = this.borderRect_!.getBoundingClientRect(); + xy = style.getPageOffset(this.borderRect_!); scaledWidth = bBox.width; scaledHeight = bBox.height; } diff --git a/core/field_colour.ts b/core/field_colour.ts index de6baf9ca27..1b022b64ccb 100644 --- a/core/field_colour.ts +++ b/core/field_colour.ts @@ -19,7 +19,12 @@ import * as browserEvents from './browser_events.js'; import * as Css from './css.js'; import * as dom from './utils/dom.js'; import * as dropDownDiv from './dropdowndiv.js'; -import {Field, FieldConfig, FieldValidator} from './field.js'; +import { + Field, + FieldConfig, + FieldValidator, + UnattachedFieldError, +} from './field.js'; import * as fieldRegistry from './field_registry.js'; import * as aria from './utils/aria.js'; import * as colour from './utils/colour.js'; @@ -168,29 +173,118 @@ export class FieldColour extends Field { this.getConstants()!.FIELD_COLOUR_DEFAULT_WIDTH, this.getConstants()!.FIELD_COLOUR_DEFAULT_HEIGHT, ); - if (!this.getConstants()!.FIELD_COLOUR_FULL_BLOCK) { - this.createBorderRect_(); - this.getBorderRect().style['fillOpacity'] = '1'; - } else if (this.sourceBlock_ instanceof BlockSvg) { - this.clickTarget_ = this.sourceBlock_.getSvgRoot(); + this.createBorderRect_(); + this.getBorderRect().style['fillOpacity'] = '1'; + this.getBorderRect().setAttribute('stroke', '#fff'); + if (this.isFullBlockField()) { + this.clickTarget_ = (this.sourceBlock_ as BlockSvg).getSvgRoot(); } } + protected override isFullBlockField(): boolean { + const block = this.getSourceBlock(); + if (!block) throw new UnattachedFieldError(); + + const constants = this.getConstants(); + return block.isSimpleReporter() && !!constants?.FIELD_COLOUR_FULL_BLOCK; + } + /** * Updates text field to match the colour/style of the block. */ override applyColour() { - if (!this.getConstants()!.FIELD_COLOUR_FULL_BLOCK) { - if (this.borderRect_) { - this.borderRect_.style.fill = this.getValue() as string; - } - } else if (this.sourceBlock_ instanceof BlockSvg) { - this.sourceBlock_.pathObject.svgPath.setAttribute( - 'fill', - this.getValue() as string, - ); - this.sourceBlock_.pathObject.svgPath.setAttribute('stroke', '#fff'); + const block = this.getSourceBlock() as BlockSvg | null; + if (!block) throw new UnattachedFieldError(); + + if (!this.fieldGroup_) return; + + const borderRect = this.borderRect_; + if (!borderRect) { + throw new Error('The border rect has not been initialized'); + } + + if (!this.isFullBlockField()) { + borderRect.style.display = 'block'; + borderRect.style.fill = this.getValue() as string; + } else { + borderRect.style.display = 'none'; + // In general, do *not* let fields control the color of blocks. Having the + // field control the color is unexpected, and could have performance + // impacts. + block.pathObject.svgPath.setAttribute('fill', this.getValue() as string); + block.pathObject.svgPath.setAttribute('stroke', '#fff'); + } + } + + /** + * Returns the height and width of the field. + * + * This should *in general* be the only place render_ gets called from. + * + * @returns Height and width. + */ + override getSize(): Size { + if (this.getConstants()?.FIELD_COLOUR_FULL_BLOCK) { + // In general, do *not* let fields control the color of blocks. Having the + // field control the color is unexpected, and could have performance + // impacts. + // Full block fields have more control of the block than they should + // (i.e. updating fill colour). Whenever we get the size, the field may + // no longer be a full-block field, so we need to rerender. + this.render_(); + this.isDirty_ = false; } + return super.getSize(); + } + + /** + * Updates the colour of the block to reflect whether this is a full + * block field or not. + */ + protected override render_() { + super.render_(); + + const block = this.getSourceBlock() as BlockSvg | null; + if (!block) throw new UnattachedFieldError(); + // In general, do *not* let fields control the color of blocks. Having the + // field control the color is unexpected, and could have performance + // impacts. + // Whenever we render, the field may no longer be a full-block-field so + // we need to update the colour. + if (this.getConstants()!.FIELD_COLOUR_FULL_BLOCK) block.applyColour(); + } + + /** + * Updates the size of the field based on whether it is a full block field + * or not. + * + * @param margin margin to use when positioning the field. + */ + protected updateSize_(margin?: number) { + const constants = this.getConstants(); + const xOffset = + margin !== undefined + ? margin + : !this.isFullBlockField() + ? constants!.FIELD_BORDER_RECT_X_PADDING + : 0; + let totalWidth = xOffset * 2; + let contentWidth = 0; + if (!this.isFullBlockField()) { + contentWidth = constants!.FIELD_COLOUR_DEFAULT_WIDTH; + totalWidth += contentWidth; + } + + let totalHeight = constants!.FIELD_TEXT_HEIGHT; + if (!this.isFullBlockField()) { + totalHeight = Math.max(totalHeight, constants!.FIELD_BORDER_RECT_HEIGHT); + } + + this.size_.height = totalHeight; + this.size_.width = totalWidth; + + this.positionTextElement_(xOffset, contentWidth); + this.positionBorderRect_(); } /** @@ -206,26 +300,6 @@ export class FieldColour extends Field { return colour.parse(newValue); } - /** - * Update the value of this colour field, and update the displayed colour. - * - * @param newValue The value to be saved. The default validator guarantees - * that this is a colour in '#rrggbb' format. - */ - protected override doValueUpdate_(newValue: string) { - this.value_ = newValue; - if (this.borderRect_) { - this.borderRect_.style.fill = newValue; - } else if ( - this.sourceBlock_ && - this.sourceBlock_.rendered && - this.sourceBlock_ instanceof BlockSvg - ) { - this.sourceBlock_.pathObject.svgPath.setAttribute('fill', newValue); - this.sourceBlock_.pathObject.svgPath.setAttribute('stroke', '#fff'); - } - } - /** * Get the text for this field. Used when the block is collapsed. * diff --git a/core/field_input.ts b/core/field_input.ts index 8c2223f7d59..8281afe343c 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -34,6 +34,7 @@ import * as userAgent from './utils/useragent.js'; import * as WidgetDiv from './widgetdiv.js'; import type {WorkspaceSvg} from './workspace_svg.js'; import * as renderManagement from './render_management.js'; +import {Size} from './utils/size.js'; /** * Supported types for FieldInput subclasses. @@ -88,7 +89,7 @@ export abstract class FieldInput extends Field< * Whether the field should consider the whole parent block to be its click * target. */ - fullBlockClickTarget_: boolean | null = false; + fullBlockClickTarget_: boolean = false; /** The workspace that this field belongs to. */ protected workspace_: WorkspaceSvg | null = null; @@ -142,37 +143,22 @@ export abstract class FieldInput extends Field< override initView() { const block = this.getSourceBlock(); - if (!block) { - throw new UnattachedFieldError(); - } - if (this.getConstants()!.FULL_BLOCK_FIELDS) { - // Step one: figure out if this is the only field on this block. - // Rendering is quite different in that case. - let nFields = 0; - let nConnections = 0; - // Count the number of fields, excluding text fields - for (let i = 0, input; (input = block.inputList[i]); i++) { - for (let j = 0; input.fieldRow[j]; j++) { - nFields++; - } - if (input.connection) { - nConnections++; - } - } - // The special case is when this is the only non-label field on the block - // and it has an output but no inputs. - this.fullBlockClickTarget_ = - nFields <= 1 && block.outputConnection && !nConnections; - } else { - this.fullBlockClickTarget_ = false; - } + if (!block) throw new UnattachedFieldError(); + super.initView(); - if (this.fullBlockClickTarget_) { + if (this.isFullBlockField()) { this.clickTarget_ = (this.sourceBlock_ as BlockSvg).getSvgRoot(); - } else { - this.createBorderRect_(); } - this.createTextElement_(); + } + + protected override isFullBlockField(): boolean { + const block = this.getSourceBlock(); + if (!block) throw new UnattachedFieldError(); + + // Side effect for backwards compatibility. + this.fullBlockClickTarget_ = + !!this.getConstants()?.FULL_BLOCK_FIELDS && block.isSimpleReporter(); + return this.fullBlockClickTarget_; } /** @@ -223,23 +209,54 @@ export abstract class FieldInput extends Field< * Updates text field to match the colour/style of the block. */ override applyColour() { - if (!this.sourceBlock_ || !this.getConstants()!.FULL_BLOCK_FIELDS) return; + const block = this.getSourceBlock() as BlockSvg | null; + if (!block) throw new UnattachedFieldError(); - const source = this.sourceBlock_ as BlockSvg; + if (!this.getConstants()!.FULL_BLOCK_FIELDS) return; + if (!this.fieldGroup_) return; - if (this.borderRect_) { - this.borderRect_.setAttribute('stroke', source.style.colourTertiary); + if (!this.isFullBlockField() && this.borderRect_) { + this.borderRect_!.style.display = 'block'; + this.borderRect_.setAttribute('stroke', block.style.colourTertiary); } else { - source.pathObject.svgPath.setAttribute( + this.borderRect_!.style.display = 'none'; + // In general, do *not* let fields control the color of blocks. Having the + // field control the color is unexpected, and could have performance + // impacts. + block.pathObject.svgPath.setAttribute( 'fill', this.getConstants()!.FIELD_BORDER_RECT_COLOUR, ); } } + /** + * Returns the height and width of the field. + * + * This should *in general* be the only place render_ gets called from. + * + * @returns Height and width. + */ + override getSize(): Size { + if (this.getConstants()?.FULL_BLOCK_FIELDS) { + // In general, do *not* let fields control the color of blocks. Having the + // field control the color is unexpected, and could have performance + // impacts. + // Full block fields have more control of the block than they should + // (i.e. updating fill colour). Whenever we get the size, the field may + // no longer be a full-block field, so we need to rerender. + this.render_(); + this.isDirty_ = false; + } + return super.getSize(); + } + /** * Updates the colour of the htmlInput given the current validity of the * field's value. + * + * Also updates the colour of the block to reflect whether this is a full + * block field or not. */ protected override render_() { super.render_(); @@ -256,6 +273,15 @@ export abstract class FieldInput extends Field< aria.setState(htmlInput, aria.State.INVALID, false); } } + + const block = this.getSourceBlock() as BlockSvg | null; + if (!block) throw new UnattachedFieldError(); + // In general, do *not* let fields control the color of blocks. Having the + // field control the color is unexpected, and could have performance + // impacts. + // Whenever we render, the field may no longer be a full-block-field so + // we need to update the colour. + if (this.getConstants()!.FULL_BLOCK_FIELDS) block.applyColour(); } /** @@ -374,7 +400,7 @@ export abstract class FieldInput extends Field< htmlInput.style.fontSize = fontSize; let borderRadius = FieldInput.BORDERRADIUS * scale + 'px'; - if (this.fullBlockClickTarget_) { + if (this.isFullBlockField()) { const bBox = this.getScaledBBox(); // Override border radius. @@ -462,8 +488,6 @@ export abstract class FieldInput extends Field< * @param _value The new value of the field. */ onFinishEditing_(_value: AnyDuringMigration) {} - // NOP by default. - // TODO(#2496): Support people passing a func into the field. /** * Bind handlers for user input on the text input field's editor.