Fzchriha/labgraph UI by fzchriha · Pull Request #71 · facebookresearch/labgraph

fzchriha · 2022-04-12T20:47:25Z

Description

Create UI for real-time messaging and set up the logic behind real-time data rendering.

Fixes #45

Demo

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)

Checklist:

New and existing unit tests pass locally with these changes?
Has code been commented, particularly in hard-to-understand areas?
Have you made corresponding changes to the documentation?

fzchriha · 2022-04-14T17:33:27Z

@bennaaym I added the test for the existence of a .env.local file
Here is how it is displayed

Please let me know if you have any other suggestions?

bennaaym · 2022-04-14T18:25:53Z

@fzchriha, maybe the message could be updated to Undefined Environment Variable: REACT_APP_WS_API, just to be more specific. otherwise looks good. great work!

fzchriha · 2022-04-18T20:50:12Z

@jfResearchEng Hi Jimmy here is my final PR, I have made all the requested changes as well as added additional UI features & tests. Can you please review my code?

navn-r

Overall, great work! 🎉 🌮

Just left some nitpicks and code suggestions 🚀. Apologies if the suggestions formatting is a bit off...the text editor on GitHub for suggestions sucks.

extensions/prototypes/labgraph_monitor/README.md

extensions/prototypes/labgraph_monitor/src/components/SettingPanel/EdgeSettings.tsx

extensions/prototypes/labgraph_monitor/src/redux/reducers/graph/mock/interfaces/IMock.ts

navn-r · 2022-04-18T23:52:03Z

extensions/prototypes/labgraph_monitor/src/redux/reducers/graph/mock/mockReducer.ts

            state.mockGraph = action.payload;
        },
+
+        setMockRealtimeData: (state, action: PayloadAction<any>) => {


same here with PayloadAction<T>

extensions/prototypes/labgraph_monitor/src/contexts/WSContext/WSContext.tsx

navn-r · 2022-04-18T23:56:28Z

extensions/prototypes/labgraph_monitor/src/components/SettingPanel/SettingPanel.tsx

+    const handleMouseUp = () => {
+        document.removeEventListener('mouseup', handleMouseUp, true);


I'm a bit confused on what's going on here. The function is handleMouseUp, and its passing itself in the event listener?

The onMouseDown and related event callbacks were made in order to not use the native document object. In React's event system, the listener is onMouseDown itself, iirc it uses the native mousedown event internally, so you don't needa use both.

extensions/prototypes/labgraph_monitor/src/components/SettingPanel/EdgeSettings.tsx

jfResearchEng · 2022-04-20T00:19:52Z

Just to confirm with you, whether this PR can be used for final review and the rest of your LabGraph UI drafts can be ignored for now?

fzchriha · 2022-04-20T13:14:41Z

Yes I confirm, the other PRs are drafts

jfResearchEng · 2022-04-20T18:31:30Z

Has this PR been rebased and included the latest changes made in PR #64 ?

fzchriha · 2022-04-20T18:53:38Z

@jfResearchEng Yes I tested my changes with Damir's latest changed

extensions/prototypes/labgraph_monitor/README.md

jfResearchEng · 2022-04-20T19:14:32Z

@jfResearchEng Yes I tested my changes with Damir's latest changed

As this is a later-launched PR (71) than PR #64, can you please rebase this PR to include the latest changes in PR #64 , for example, extensions/yaml_support/labgraph_monitor/examples/labgraph_monitor_example.py has been added in #64 . The documentation would also need to be updated accordingly.

jfResearchEng

Please rebase this PR with latest updates from PR64 so that an end-to-end test can be conducted by the reviewer(s).

fzchriha · 2022-04-22T14:48:19Z

@jfResearchEng I updated the README file with the latest changes from PR64

extensions/prototypes/labgraph_monitor/README.md

jfResearchEng · 2022-04-22T15:16:25Z

extensions/prototypes/labgraph_monitor/README.md


 ```
-yarn start
+labgraph\extensions\prototypes\labgraph_monitor> yarn start


An empty UI has launched after running this line.
Reference: https://pasteboard.co/K6noFCNDD7A3.png

Do you see the icon on the right?

jfResearchEng

Merging this PR for now and for the other improvements, please help create a new PR.

jfResearchEng · 2022-04-26T00:15:38Z

Thank you for contributing to all the PR improvements on LabGraph Monitor!

jfResearchEng · 2022-05-01T18:32:01Z

@fzchriha, is this still an open issue: #69 ?

Setup real time messaging and handle errors

1a2af23

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 12, 2022

fzchriha changed the title ~~Setup real time messaging and handle errors~~ Fzchriha/labgraph UI Apr 12, 2022

dtemir mentioned this pull request Apr 14, 2022

WebSocket Connection Interrupted on LabGraph Monitor #69

Open

Mock Realtime Data

41cd833

fzchriha added 5 commits April 14, 2022 15:30

UI: Make Panel Resizable

fd8200f

Update Error Alert

f1d55ed

Update README

12256a4

UI: adjust Tabs size

2aaaa05

Fix lint issues

ab92ab0

navn-r reviewed Apr 19, 2022

View reviewed changes

jfResearchEng reviewed Apr 20, 2022

View reviewed changes

extensions/prototypes/labgraph_monitor/README.md Outdated Show resolved Hide resolved

jfResearchEng suggested changes Apr 21, 2022

View reviewed changes

fzchriha added 2 commits April 21, 2022 15:50

Code Review implemetation

8751c80

Rebase Doc with latest changes PR64

330c763

fzchriha requested a review from jfResearchEng April 21, 2022 20:08

jfResearchEng reviewed Apr 22, 2022

View reviewed changes

extensions/prototypes/labgraph_monitor/README.md Show resolved Hide resolved

jfResearchEng reviewed Apr 22, 2022

View reviewed changes

Alert test & error catch

820f365

fzchriha requested a review from jfResearchEng April 22, 2022 19:22

jfResearchEng approved these changes Apr 26, 2022

View reviewed changes

jfResearchEng merged commit 6c391b4 into facebookresearch:main Apr 26, 2022

		const handleMouseUp = () => {
		document.removeEventListener('mouseup', handleMouseUp, true);

Conversation

fzchriha commented Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Demo

Type of change

Checklist:

Uh oh!

fzchriha commented Apr 14, 2022

Uh oh!

bennaaym commented Apr 14, 2022

Uh oh!

fzchriha commented Apr 18, 2022

Uh oh!

navn-r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

navn-r Apr 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

navn-r Apr 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jfResearchEng commented Apr 20, 2022

Uh oh!

fzchriha commented Apr 20, 2022

Uh oh!

jfResearchEng commented Apr 20, 2022

Uh oh!

fzchriha commented Apr 20, 2022

Uh oh!

Uh oh!

jfResearchEng commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jfResearchEng left a comment

Choose a reason for hiding this comment

Uh oh!

fzchriha commented Apr 22, 2022

Uh oh!

Uh oh!

jfResearchEng Apr 22, 2022

Choose a reason for hiding this comment

Uh oh!

fzchriha Apr 22, 2022 • 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!

jfResearchEng left a comment

Choose a reason for hiding this comment

Uh oh!

jfResearchEng commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jfResearchEng commented May 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fzchriha commented Apr 12, 2022 •

edited

Loading

jfResearchEng commented Apr 20, 2022 •

edited

Loading

fzchriha Apr 22, 2022 •

edited

Loading

jfResearchEng commented Apr 26, 2022 •

edited

Loading