-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improves the UI for graph #60
Conversation
- only one node is colored - The others are higlighted - Edge paths are highlighted too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested.
- Reviewed the entire pull request up to 940bc64
- Looked at
251
lines of code in2
files - Took 2 minutes and 8 seconds to review
More info
- Skipped
0
files when reviewing. - Skipped posting
0
additional comments because they didn't meet confidence threshold of50%
.
Workflow ID: wflow_jixzOIyMuHRqkSEf
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. We'll respond in a few minutes. Learn more here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me!
- Performed an incremental review on 2058b94
- Looked at
68
lines of code in3
files - Took 2 minutes and 16 seconds to review
More info
- Skipped
0
files when reviewing. - Skipped posting
2
additional comments because they didn't meet confidence threshold of50%
.
1. telemetry/ui/src/components/routes/app/GraphView.tsx:122
:
- Assessed confidence :
0%
- Comment:
Good job on removing the console.log statements. It makes the code cleaner and removes unnecessary console output. - Reasoning:
The PR author has removed some console.log statements which were probably used for debugging. This is a good practice as it cleans up the code and removes unnecessary console output.
2. telemetry/ui/src/components/routes/app/AppView.tsx:207
:
- Assessed confidence :
0%
- Comment:
Good job on replacing '==' with '==='. This prevents unexpected behavior by also checking the type of the operands. - Reasoning:
The PR author has replaced the '==' operator with '===' in a comparison. This is a good practice as '===' also checks the type of the operands, which can prevent unexpected behavior.
Workflow ID: wflow_eDacxfmOjqzFdF1T
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
2058b94
to
21283ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me!
- Performed an incremental review on 21283ca
- Looked at
71
lines of code in2
files - Took 2 minutes and 7 seconds to review
More info
- Skipped
1
files when reviewing. - Skipped posting
2
additional comments because they didn't meet confidence threshold of50%
.
1. telemetry/ui/src/components/common/layout.tsx:16
:
- Assessed confidence :
100%
- Grade:
0%
- Comment:
Consider using '===' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
if (mode === 'first-minimal') {
- Reasoning:
The comparison operator '==' is used in the code. It's a good practice to use '===' instead of '==' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
2. telemetry/ui/src/components/common/table.tsx:132
:
- Assessed confidence :
100%
- Grade:
0%
- Comment:
Consider using '===' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
dense === 0 ? 'py-4' : dense === 1 ? 'py-2.5' : 'py-0.2',
- Reasoning:
The comparison operator '==' is used in the code. It's a good practice to use '===' instead of '==' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
Workflow ID: wflow_HRCMFLFYBmg8UCvf
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
21283ca
to
b33d95c
Compare
Nothing in the python version but this has some UI updates that'll be released shortly, and they're packaged up on pypi as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me!
- Performed an incremental review on b33d95c
- Looked at
71
lines of code in2
files - Took 2 minutes and 42 seconds to review
More info
- Skipped
1
files when reviewing. - Skipped posting
2
additional comments because they didn't meet confidence threshold of50%
.
1. telemetry/ui/src/components/common/layout.tsx:16
:
- Assessed confidence :
100%
- Grade:
0%
- Comment:
Consider using '===' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
if (mode === 'first-minimal') {
- Reasoning:
The comparison operator '==' is used in the code. It's a good practice to use '===' instead of '==' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
2. telemetry/ui/src/components/common/table.tsx:132
:
- Assessed confidence :
100%
- Grade:
0%
- Comment:
Consider using '===' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
dense === 0 ? 'py-4' : dense === 1 ? 'py-2.5' : 'py-0.2',
- Reasoning:
The comparison operator '==' is used in the code. It's a good practice to use '===' instead of '==' for comparison to avoid unexpected type coercion. The '===' operator checks both value and type, which is considered more correct.
Workflow ID: wflow_sIvQw0QfXVx6j5SC
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
Skipped PR review on c27e2ee because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away. Generated with ❤️ by ellipsis.dev |
Skipped PR review on ff5c4b4 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away. Generated with ❤️ by ellipsis.dev |
Summary:
This PR updates the graph UI by enhancing node and edge highlighting, incorporating edge data into the graph layout, and making updates to several files.
Key points:
AppView.tsx
andGraphView.tsx
hoverIndex
state tohoverSequenceID
inAppView.tsx
EdgeData
type inGraphView.tsx
getLayoutedElements
andconvertApplicationToGraph
functions inGraphView.tsx
to include edge datalayout.tsx
andtable.tsx
files intelemetry/ui/src/components/common
directoryGenerated with ❤️ by ellipsis.dev