UI rewrite #117

calebho · 2019-12-25T02:27:40Z

Creating a preliminary PR to track UI progress. The idea is to rewrite the UI using industry-standard browser technologies instead of fiddling around with tkinter.

calebho · 2019-12-25T02:29:31Z

To get this running,

cd app
npm i
npm run start

calebho · 2019-12-25T02:34:26Z

Current demo: https://streamable.com/o0okk

keggsmurph21 · 2019-12-26T17:58:09Z

app/.eslintrc.yml

+  - react
+  - '@typescript-eslint'
+rules:
+  sort-imports: off


I'm running into an annoying issue where imports of the form

import Default, { something } from 'foo';

are a lint error because braced imports are supposed to occur before default imports. After fiddling around with the settings a bit, I gave up because it wasn't worth the effort. If you have any ideas I'm all ears.

keggsmurph21 · 2019-12-26T17:59:17Z

app/.prettierrc

@@ -0,0 +1,6 @@
+{
+  "tabWidth": 2,


I know that 2 is the default for javascript, but how would you feel about using 4 here to be more consistent with the rest of the repo?

I have a weak preference for 2. It's pretty common for multi-language projects to have separate styles for each language

keggsmurph21 · 2019-12-26T18:00:13Z

app/package.json

+  "scripts": {
+    "build": "webpack --config ./webpack.config.js",
+    "start": "npm run build && electron ./dist/main.js",
+    "test": "echo \"Error: no test specified\" && exit 1"


Is this "coming soon" ?

keggsmurph21 · 2019-12-26T18:01:36Z

app/src/main/main.ts

+    width: 800,
+    height: 600,


The default window size needs to be bumped up a bit. This was the first render when I ran the app:

keggsmurph21 · 2019-12-26T18:05:10Z

app/src/renderer/index.html

+  </head>
+  <body>
+    <div id="app">
+      <h1>Hello World!</h1>


No reason to keep this here ...

keggsmurph21 · 2019-12-26T18:05:28Z

app/src/renderer/index.html

+<html>
+  <head>
+    <meta charset="UTF-8">
+    <title>Hello World!</title>


Suggested change

<title>Hello World!</title>

<title>UltraTrace</title>

:^)

keggsmurph21

Overall this looks really good. I haven't reviewed the React stuff since I don't have enough experience with that yet. If there's anything that you want me to look at in particular, just let me know and I can dive in.

A few thoughts:

I encountered these warnings (although they feel more like errors) when running the app ... not sure if these are things you've come across

react-dom.development.js:530 Warning: validateDOMNesting(...): <th> cannot appear as a child of <tfoot>.
    in th (created by TableCell)
    in TableCell (created by TableHeaderCell)
    in TableHeaderCell (created by TraceSelector)
    in tfoot (created by TableHeader)
    in TableHeader (created by TableFooter)
    in TableFooter (created by TraceSelector)
    in table (created by Table)
    in Table (created by TraceSelector)
    in TraceSelector (created by Sidebar)
    in div (created by Segment)
    in Segment (created by Sidebar)
    in Sidebar (created by Responsive)
    in Responsive (created by App)
    in div (created by SegmentGroup)
    in SegmentGroup (created by App)
    in App
warningWithoutStack	@	react-dom.development.js:530

react.development.js:167 Warning: Each child in a list should have a unique "key" prop.

Check the render method of `Annotation`. See https://fb.me/react-warning-keys for more information.
    in Point (created by Annotation)
    in Annotation (created by withRelativeMousePos(undefined))
    in withRelativeMousePos(undefined) (created by IsMouseHovering(withRelativeMousePos(undefined)))
    in IsMouseHovering(withRelativeMousePos(undefined)) (created by Window)
    in div (created by Segment)
    in Segment (created by Window)
    in Window (created by Responsive)
    in Responsive (created by App)
    in div (created by SegmentGroup)
    in SegmentGroup (created by App)
    in App
warningWithoutStack @ react.development.js:167

Warning: Unsupported style property background-color. Did you mean backgroundColor?
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div (created by Point)
    in Point (created by Annotation)
    in div (created by styled.div)
    in styled.div (created by Annotation)
    in div (created by styled.div)
    in styled.div (created by Annotation)
    in Annotation (created by withRelativeMousePos(undefined))
    in withRelativeMousePos(undefined) (created by IsMouseHovering(withRelativeMousePos(undefined)))
    in IsMouseHovering(withRelativeMousePos(undefined)) (created by Window)
    in div (created by Segment)
    in Segment (created by Window)
    in Window (created by Responsive)
    in Responsive (created by App)
    in div (created by SegmentGroup)
    in SegmentGroup (created by App)
    in App
warningWithoutStack @ react-dom.development.js:530

I assume you've already thought about this, but at some point we'll want to package our app into one runnable file.
Have you thought about how the IPC will work? Will the electron-main process spawn a Python "server" ?

keggsmurph21 · 2019-12-26T18:09:20Z

app/src/renderer/types.d.ts

@@ -0,0 +1,94 @@
+declare module '*.jpg';


I'm not sure what this line does -- why do we need to declare static images?

When you do something like

import image from 'someimage.jpg';

Typescript will complain because it doesn't know what someimage.jpg is (I believe it looks at TS and JS files in your project directory + node_modules).

Anyway this is temporary until we get real images

keggsmurph21 · 2019-12-26T18:12:28Z

app/webpack.config.js

+    target: 'electron-renderer',
+    devtool: 'source-map',
+    resolve: {
+      // Add '.ts' and '.tsx' as resolvable extensions.


Stale comment?

app/webpack.config.js

calebho · 2020-01-03T04:12:14Z

I encountered these warnings (although they feel more like errors) when running the app ... not sure if these are things you've come across

react-dom.development.js:530 Warning: validateDOMNesting(...): <th> cannot appear as a child of <tfoot>.
    in th (created by TableCell)
    in TableCell (created by TableHeaderCell)
    in TableHeaderCell (created by TraceSelector)
    in tfoot (created by TableHeader)
    in TableHeader (created by TableFooter)
    in TableFooter (created by TraceSelector)
    in table (created by Table)
    in Table (created by TraceSelector)
    in TraceSelector (created by Sidebar)
    in div (created by Segment)
    in Segment (created by Sidebar)
    in Sidebar (created by Responsive)
    in Responsive (created by App)
    in div (created by SegmentGroup)
    in SegmentGroup (created by App)
    in App
warningWithoutStack	@	react-dom.development.js:530

react.development.js:167 Warning: Each child in a list should have a unique "key" prop.

Check the render method of `Annotation`. See https://fb.me/react-warning-keys for more information.
    in Point (created by Annotation)
    in Annotation (created by withRelativeMousePos(undefined))
    in withRelativeMousePos(undefined) (created by IsMouseHovering(withRelativeMousePos(undefined)))
    in IsMouseHovering(withRelativeMousePos(undefined)) (created by Window)
    in div (created by Segment)
    in Segment (created by Window)
    in Window (created by Responsive)
    in Responsive (created by App)
    in div (created by SegmentGroup)
    in SegmentGroup (created by App)
    in App
warningWithoutStack @ react.development.js:167

Warning: Unsupported style property background-color. Did you mean backgroundColor?
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div (created by Point)
    in Point (created by Annotation)
    in div (created by styled.div)
    in styled.div (created by Annotation)
    in div (created by styled.div)
    in styled.div (created by Annotation)
    in Annotation (created by withRelativeMousePos(undefined))
    in withRelativeMousePos(undefined) (created by IsMouseHovering(withRelativeMousePos(undefined)))
    in IsMouseHovering(withRelativeMousePos(undefined)) (created by Window)
    in div (created by Segment)
    in Segment (created by Window)
    in Window (created by Responsive)
    in Responsive (created by App)
    in div (created by SegmentGroup)
    in SegmentGroup (created by App)
    in App
warningWithoutStack @ react-dom.development.js:530

Yeah these are coming from third-party components. Will investigate on my final pass.

I assume you've already thought about this, but at some point we'll want to package our app into one runnable file.

Yup that's the plan

Have you thought about how the IPC will work? Will the electron-main process spawn a Python "server" ?

Yeah so the main process will be pretty light since our backend is in Python. It'll just handle spawning the Python server and just join on the process handle.

Initial sidebar + annotation window

a38f296

calebho requested a review from keggsmurph21 December 25, 2019 02:27

keggsmurph21 reviewed Dec 26, 2019

View reviewed changes

Misc component updates; Add custom semantic-ui theming

178746a

jonorthwash force-pushed the master branch 3 times, most recently from 81d5a96 to 9d1c199 Compare October 13, 2020 19:46

UI rewrite #117

Are you sure you want to change the base?

UI rewrite #117

Uh oh!

Conversation

calebho commented Dec 25, 2019

Uh oh!

calebho commented Dec 25, 2019

Uh oh!

calebho commented Dec 25, 2019

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!

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!

keggsmurph21 left a comment

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!

Uh oh!

calebho commented Jan 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants