search

playcanvas / pcui

UI component library for web-based tools
http://playcanvas.github.io/pcui
MIT License
664 stars 62 forks source link

Default tabIndex value fix #270

Closed ellthompson closed 1 year ago

ellthompson commented 1 year ago

The args object should not be modified before it is copied otherwise the following error is thrown:

Uncaught TypeError: Cannot add property tabIndex, object is not extensible
    at new BooleanInput (index.js:37344:23)

This PR first copies the args object before updating this value., following the example in the Panel class.

willeastcott commented 1 year ago

I just tried and failed to reproduce that error in the booleaninput.html example. How do I get this to happen?

ellthompson commented 1 year ago

This error was thrown in the examples browser using the latest version of PCUI, which is processed using the typescript rollup plugin.

willeastcott commented 1 year ago

Just to clarify here - you mean the PlayCanvas Engine Examples Browser (this repo has an examples browser too now). :smile:

So why would that app do that, but not the PCUI examples browser (which is vanilla ESM)?

willeastcott commented 1 year ago

Was preventExtensions called in the compiled code perhaps?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cant_define_property_object_not_extensible

ellthompson commented 1 year ago

Yep the engine examples browser. It doesn't seem to call preventExtensions anywhere though.

willeastcott commented 1 year ago

Have you traced through the compiled JS code to see what's going on? There genuinely shouldn't be anything wrong with the code you're changing here, as far as I can tell.

ellthompson commented 1 year ago

It looks like this issue was caused by the LabelGroup component passing react component properties directly into the constructor of the LabelGroup field. These properties should be copied to a mutable object beforehand. I've included that fix as part of the LabelGroup change here, so I'm closing this issue.