-
Notifications
You must be signed in to change notification settings - Fork 29
Toolbar with Breadcrumbs, improved Start/Stop and more #2098
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
Toolbar with Breadcrumbs, improved Start/Stop and more #2098
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2098 +/- ##
========================================
- Coverage 73.5% 73.5% -0.1%
========================================
Files 452 452
Lines 16704 16704
Branches 1661 1661
========================================
- Hits 12282 12280 -2
- Misses 3969 3971 +2
Partials 453 453
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
go go go! (NICE)
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.
Every iteration looks better and better
I enjoy a lot seeing how much data is getting displayed and arranged in the UI
well done
PS: i cannot really help by giving meaningful feedback with the code and leave that for more capable reviewers
for (let nodeId in slideShow) { | ||
const node = slideShow[nodeId]; | ||
nodes.push({ | ||
...node, |
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.
what are those ..., for?
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.
}); | ||
} | ||
nodes.sort((a, b) => (a.position > b.position) ? 1 : -1); | ||
const nodeIds = []; |
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.
strange language that defines a const
can change it :-) I guess const prevent assignment but not chancing the content of the container. ... still strange :-)
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.
It is a const
in the sense that can't be reassigned, but it is mutable:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const
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.
pretty nice!!
What do these changes do?
Bonus
New toolbar:

Stop/Run (workbench):

Stop/Run (guided):

Quality by clicking the stars on card:

Related issue/s
related to ITISFoundation/osparc-issues#327
How to test
Checklist