engageLively / galyleo-dashboard

3 stars 2 forks source link

No Error Checking for Morphs #70

Open rickmcgeer opened 3 years ago

rickmcgeer commented 3 years ago

I've got a test dashboard (https://github.com/engageLively/galyleo-test-dashboards/blob/main/mtbf_mttr_dashboard.gd.json) that illustrates the problem. Mouse over the bubble charts and the pointer is always a hand, which shows that the chart (Google) is grabbing the mouse event. Physical editing for us requires mouse actions (we need to get a halo, manipulate control points, etc) and ATM all we can do it move the charts around; we don't get a halo, and we can't manipulate the corners, etc.

rickmcgeer commented 3 years ago

This appears to be morphic properties of charts not getting saved/restored (opacity was set to false!). Save the morphic properties for the chart and this should take care of it. Use the test dashboard in comment #1 ro check

rickmcgeer commented 3 years ago

I have now put error checking in for simple morphic and text properties and tested them. The following changes are in:

  1. I modified _morphicFields_ and _textFields_ to return a list of triples {name, validCheck, default}, where name is the field name, validCheck is a function which takes in a value for the field name and returns true/false if the value is valid/invalid for that field, and default is the value is validCheck is false. See $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_morphicFields_","type":"class-instance-getter"}]}); and $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_morphicFields_","type":"class-instance-getter"}]});
  2. I added a new method _getFieldValue_(source, field) where source is a data source containing field.name and field is a record returned by _morphicFields_ or _textFields_. __getField__ accesses the field in source, checks to see if the value is valid. If it is returns the value, if not it returns field.default. See $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_getFieldValue_","type":"class-instance-method"}]});
  3. I modified _getFields_ to use _getFieldValue_ replacing result[field] = object[field] with result[field.name] = this._getFieldValue_(object, field). See: $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_getFields_","type":"class-instance-method"}]});
  4. I modified _setFields_ to use _getFieldValue_ replacing morph[field] = descriptor[field] with morph[field.name] = this.__getField__(descriptor, field)). See $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_setFields_","type":"class-instance-method"}]});

Still to do (by me) fix the complexProperties methods: _setComplexFields_, _setComplexTextFields_, _complexMorphicFields_, _complexTextFields_. The issue here is how to extend the simple scalar-based approach we've done to more complex structures and functions. Assigning to @merryman for comment and discussion, then will take back to implement at @merryman 's discretion.

rickmcgeer commented 3 years ago

I've now fixed _setComplexTextFields_ and (mostly) _setComplexFields_. On the latter, there is still an issue with borders, and @merryman needs to weigh in on this. The issue is that the inspector shows borders as four fields (borderColor, borderRadius, borderWidth, borderStyle), whereas the spec has a single field (border) with fields (color, borderRadius, width, style), each of which has four sides (top, bottom, left, right). Writing a decent intermediate form spec for this is tough, not to mention checking code. Once this is settled, documented and implemented (and a test dashboard built and tested), we can cliose this

Updates to _setComplexFields_. Added a method _returnValidPoint_ which returns the point corresponding to the JSON description if valid, default if not. Used this for position, extent, origin. See: $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_setComplexFields_","type":"class-instance-method"}]});

Updates to _setComplexTextFields_: added a default color for fill. See $world.execCommand("open browser", {moduleName: "Dashboard/index.js", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"_setComplexTextFields_","type":"class-instance-method"}]});

rickmcgeer commented 2 years ago

Transfer to Jira