Move the dashboard's component library from antd to material-ui#1776
Move the dashboard's component library from antd to material-ui#1776
Conversation
run yarn add @material/core run yarn add @material-ui/icons Add the Roboto font
Make success rate mini chart into a dot
Move MetricsTable to use BaseTable
Pass props down to get breadcrumb rendering
Add authority Add path
Fix icons in Nav Add fixed width to navbar and content so that we don't get odd hanging navbar at end
Get rid of all uses of antd Spin
Add Accordion module so that we can have expandable page sections Remove the ant collapse, replace with added Accordion module Move tooltip and icon over to materialUI versions
Use it to implement error banner for ErrorModal
Also delete old versions of Tables
Add a rowKey function to BaseTable
Add material type metric table to the cards
…alid html somewhere
Add a Stepper component for the CTA
Small import and table sort tweaks
…cause it's not used anywhere
siggy
left a comment
siggy
left a comment
There was a problem hiding this comment.
This all looks super awesome! Most of my comments are nits and polish and can be deferred for subsequent PRs, some of these also may be existing issues prior to this PR.
Headers don't quite match
Bottom of sidebar does not go to bottom of page
Tables on Overview extend beyond right side of page
Tooltip on ServiceMesh page sometimes snaps to upper left corner
Hard reload on Namespaces page render unexpanded sidebar
Octopus arm alignment
Tap filter alignment
Tap tooltips not clickable, they disappear once you move your mouse
Incorrect URLs in Tap table
| { | ||
| steps.map((step, i) => { | ||
| const props = {}; | ||
| props.completed = i < steps.length - 1; // select the last step as the currently active one |
There was a problem hiding this comment.
nit:
const lastStep = _.size(steps) - 1; // hardcode the last step as the active step
...
activeStep={lastStep}
...
props.completed = i < lastStep; // select the last step as the currently active one| </React.Fragment> | ||
|
|
||
| ); | ||
| })} |
klingerf
left a comment
klingerf
left a comment
There was a problem hiding this comment.
⭐️ 🎈 Awesome work! I agree that the small style stuff can be fixed in follow up branches, and I didn't see anything else that needs immediate attention.
dadjeibaah
left a comment
dadjeibaah
left a comment
There was a problem hiding this comment.
Thanks for doing this @rmars, it looks amazing! 😻 I left a couple of comments.
web/app/js/components/Navigation.jsx
Outdated
| <div className={classes.root}> | ||
| <AppBar | ||
| position="absolute" | ||
| className={classNames(classes.appBar, this.state.drawerOpen && classes.appBarShift)}> |
There was a problem hiding this comment.
More of a readability nit: I am not entirely sure how classNames interprets statements like this.state.drawerOpen && classes.appBarShift but I found it a little difficult to comprehend. I tried rewriting the statement like so:
className={classNames(classes.appBar, {[`${classes.appBarShift}`]: this.state.drawerOpen})}>And that seemed to work the same way.
There was a problem hiding this comment.
Hahahah, what that line is actually doing is a combination of cool es6 tricks:
First, it's using the && operand to return the string name of the class if drawerOpen is true (docs).
(The js && operand will return the latter of the provided values if the first expression is truthy). Example:
>> false && "foo"
>> false
>> true && "foo"
>> "foo"
Then, it's using the fancy object assignment to assign appBarShift's className to the object if provided:
>> let a = "a_string";
>> let obj = { a }
>> obj
{a: "a_string"}
So this is equivalent to providing an object whose key is classes.appBarShift and value is this.state.drawerOpen as you've written. I'll rewrite it to be less trick-y!
There was a problem hiding this comment.
Oh my gosh! so much sorcery! That is pretty cool. it's a TIOLI so feel free to do whatever you think is best.
There was a problem hiding this comment.
Hahaha, yeah I think readable > cool, so I'll change them to more traditional args.
It's a pattern I see popping up a lot in the material examples, so maybe we can change later.
This replaces a couple of the MaterialUI icons introduced in #1776 with their original counterparts in Font Awesome, but wrapped in a MaterialUI `Icon` tag. Also fix Linkerd logo padding in sidebar. Part of #1781. Signed-off-by: Andrew Seigner <[email protected]>
This replaces a couple of the MaterialUI icons introduced in #1776 with their original counterparts in Font Awesome, but wrapped in a MaterialUI `Icon` tag. Also fix Linkerd logo padding in sidebar. Part of #1781. Signed-off-by: Andrew Seigner <[email protected]>
4 participants
Switch the component library from antd to material-ui.
This branch changes all uses of antd components to their closest equivalent in material. There is still a lot of polish that needs to go into the look of individual components, but since the major component rewrites are done, I think it's time to put tis up for review and get it in so that further work can be done in smaller branches.
Changes in this branch
package.json(mostly removing^)Follow-up work
Here is a list of known UI polish in the current branch (I will ticket each one, as well as the results of code-review of this branch, to be done in follow up branches):
index.jsshould be moved into its own file as we expand itFixes #1408