Led integration between webview and python api#192
Conversation
…github.com/microsoft/vscode-python-devicesimulator into users/t-anmah/frontend-backend-integration
| super(props); | ||
| } | ||
| componentDidMount() { | ||
| const svgElement = this.svgRef.current; |
There was a problem hiding this comment.
null check missing this.svgRef && this.svgRef.current
There was a problem hiding this comment.
I've typed svgRef as always defined since I create it in the same class before the component mounts
| } | ||
| } | ||
| componentDidUpdate() { | ||
| if (this.svgRef.current) { |
There was a problem hiding this comment.
null check missing this.svgRef
There was a problem hiding this comment.
same here, svgRef should always be defined
| ) => { | ||
| for (let j = 0; j < leds.length; j++) { | ||
| for (let i = 0; i < leds[0].length; i++) { | ||
| const ledElement = ledRefs[j][i].current; |
There was a problem hiding this comment.
I would do a null check for ledRefs here just to be sure
| } | ||
| } | ||
| }; | ||
| const setupLed = (ledElement: SVGRectElement, brightness: number) => { |
There was a problem hiding this comment.
nit: typo SVGReactElement, can you update all the instances
There was a problem hiding this comment.
SVGRectElement are referencing to tsx svg rect elements. For example,
| @@ -0,0 +1,9 @@ | |||
| interface vscode { | |||
| postMessage(message: any): void; | |||
There was a problem hiding this comment.
Is it worth typifying this?
There was a problem hiding this comment.
this interface links to the vscode api which has any type. The only place where we call this specific interface is in sendMessage (with generic typing in the following comment you made)
|
|
||
| message = create_message(state, device_name) | ||
|
|
||
| if state != previous_state: |
There was a problem hiding this comment.
Thanks for addressing the comment!
But now this part might not work as expected, right?
I would take the state updating logic and put it in this method (or in a function and call that function here).
There was a problem hiding this comment.
Thanks! I just abstracted out the state updating and made the process two function calls :).
There was a problem hiding this comment.
Awesome!
One more thing, now send_to_simulator and debug_show mutate the input parameters. It would be easier for a developer if they can use these functions without worrying about any unintended consequences.
There was a problem hiding this comment.
ok, tried to fix it in the last commit :)
There was a problem hiding this comment.
Almost perfect!
Now this check will always return true if state != previous_state:. Perhaps use state_copy instead of state?
There was a problem hiding this comment.
*nit*
I would change the name to updated_state from state_copy. Up to you!
|
Looks good. Amazing work. 💯 |
…github.com/microsoft/vscode-python-devicesimulator into users/t-anmah/frontend-backend-integration
5 participants
Description:
Python code is compiled and can send messages to typescript api then to the webview.
Type of change
Please delete options that are not relevant.
Limitations:
Please describe limitations of this PR
Testing:
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist:
npm run formatand passes the checks innpm run check