fix: Fix regressions in `Field`. by gonfunko · Pull Request #9011 · RaspberryPiFoundation/blockly

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

gonfunko · 2025-05-07T20:44:08Z

The basics

I validated my changes

The details

Resolves

Fixes#9005

Proposed Changes

This PR fixes several regressions in Field introduced by an earlier change:

It restores the Field.NBSP constant, which was in fact in use
It makes empty input fields have a reasonable minimum width
It prevents sequential spaces from collapsing when rendering a field

cpcallen · 2025-05-08T00:56:57Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


Styleguide says "At least one accessor for a property must be non-trivial: do not define "pass-through" accessors only for the purpose of hiding a property."
I gather you are doing this in order to be able to write super.size_ = newValue in FieldInput's get size_ accessor, but I think a better approach might be to have updateSize_ enforce a minimum width (possibly in Field but preferably in FieldInput). (Alas, updateSize_ isn't written in a way that makes it easy to override for this particular purpose.)

As you noted, updateSize_() is not well-suited to being overridden, and short of introducing a setSizeWithoutSideEffects() method I couldn't see a way to refactor it to be more amendable. Likewise, overriding getSize() (or calling it from updateSize_() instead of accessing the size_ property directly) is problematic due to its side effects of rerendering the field. Hence this seemed like the least-bad option, as it preserves backwards compatibility and allows for subclasses to adjust the size as they need. If you have an idea for how updateSize_() might be refactored I'd certainly be open to that though!

I had to think about this for a bit, but I think I see a way to do this without needing to have accessors on Field.
The obvious approach is to have the get and set accessors on the subclasses that want to enforce a minimum width only, and leave Field alone. As you doubtless noticed, the reason this doesn't work is that the Field constructor sets .size_, and that results in (e.g.) FieldInput instances having a .size_ property that is found before the get size_/set size_ accessors on FieldInput.prototype.
But if the Field constructor either didn't set that property if there were accessors, like so:
classFieldInput{constructor(){this.size_??=newSize(0,0);}}
or if the FieldInput constructor removed it, like so:
classFieldInput{constructor(){super();deletethis.size_;}}
then I think it's possible to have the behaviour you have here without the mostly-useless accessors on Field.prototype. Both approaches are a bit kludgy, and the delete approach might have some performance consequences (it's generally preferable not to change the shape of an object, though here at least it's being done at construction time). I'm genuinely not sure either is clearly preferable to your proposal, but I offer them for your consideration.

Additionally, I think that even if you keep the get/set accessors on Field it might be preferable to not call them from the corresponding accessors on FieldInput, either by making size protected (so that that FieldInput's accessors can use it directly) or by having FieldInput keep it's own separate Size property. A pair of accessors ought to be interchangeable with a data property, in my view, but you have revealed that that isn't always the case. I'd like to at least not tie ourselves too tightly to using accessors everywhere.

I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

cpcallen · 2025-05-08T01:03:18Z

(Restoration of .NBSP and space-to-NBSP anti-collapse code all LGTM, though.)

cpcallen · 2025-05-09T14:19:56Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


Style guide suggests something like sizeInternal:
Suggested change
private size: Size=newSize(0,0);
privatesizeInternal: Size=newSize(0,0);

I am confused about this, why? it's already private, why would we also add the word internal

Quoting the Styleguide section on get and set accesors says:
If an accessor is used to hide a class property, the hidden property may be prefixed or suffixed with any whole word, like internal or wrapped.
They do not provide a rational, but I suspect the reason is to avoid clashing with the name of the accessors. In this case the accessors are named size_ (which, with the trailing underscore, is contrary to current naming policy) but if it were size then the internal property would need to be named something else.

maribethb · 2025-05-13T21:20:13Z

+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);
+
+ /**
+ * Gets the size of this field. Because getSize() and updateSize() have side
+ * effects, this acts as a shim for subclasses which wish to adjust field
+ * bounds when setting/getting the size without triggering unwanted rendering
+ * or other side effects. Note that subclasses must override *both* get and
+ * set if either is overridden; the implementation may just call directly
+ * through to super, but it must exist per the JS spec.
+ */
+ protected get size_(): Size {
+ return this.size;
+ }
+
+ /**
+ * Sets the size of this field.
+ */
+ protected set size_(newValue: Size) {
+ this.size = newValue;
+ }


I prefer the mostly-useless accessors (which most fields can just ignore) over the kludgy approaches mentioned above.

maribethb · 2025-05-13T21:21:14Z

- protected size_: Size;
+
+ /** This field's dimensions. */
+ private size: Size = new Size(0, 0);


I am confused about this, why? it's already private, why would we also add the word internal

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

fix: Fix regressions in Field.
f84365b

gonfunko requested a review from cpcallen May 7, 2025 20:44

gonfunko assigned cpcallen May 7, 2025

gonfunko requested a review from a team as a code owner May 7, 2025 20:44

github-actionsBot added the PR: fix Fixes a bug label May 7, 2025

cpcallen reviewed May 8, 2025
View reviewed changes

cpcallen reviewed May 9, 2025
View reviewed changes

maribethb self-assigned this May 13, 2025

maribethb approved these changes May 13, 2025
View reviewed changes

gonfunko merged commit 14e1ef6 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the field-regression branch May 13, 2025 21:26

mikeharv mentioned this pull request May 15, 2025
RangeError: Maximum call stack size exceeded when using FieldNumber or FieldTextInput due to recursive size_ setter #9055
Closed
1 task

	private size: Size=newSize(0,0);
	privatesizeInternal: Size=newSize(0,0);

Uh oh!

Conversation

gonfunko commented May 7, 2025

The basics

The details

Resolves

Proposed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpcallenMay 9, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cpcallen commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gonfunko commented May 7, 2025

The basics

The details

Resolves

Proposed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpcallenMay 9, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cpcallen commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

cpcallenMay 9, 2025•
edited
Loading

cpcallenMay 9, 2025•
edited
Loading

cpcallenMay 9, 2025•
edited
Loading