Add Radial gauges and search components #19
Conversation
| 0 | ||
| </text> | ||
| </g> | ||
| <text style="fill: #404040;font-weight: 600;" dy=".35em" text-anchor="middle" transform="translate(50, 85)"> |
There was a problem hiding this comment.
I don't think the label should be baked into the SVG.
There was a problem hiding this comment.
This is copy & paste code but 100% agree!
| @@ -0,0 +1,10 @@ | |||
| <svg height="95" width="100"> | |||
There was a problem hiding this comment.
Confused why these values aren't both 100.
There was a problem hiding this comment.
Ahh, I think its because on the actual fleet code, they are scaling due to viewport size. Im actually just copying the rendered code from inspector tool. SVG is really good for this. https://css-tricks.com/scale-svg/
markup/component/search.html
Outdated
| @@ -0,0 +1,3 @@ | |||
| <div class="search-container"> | |||
| <input class="search-input" type="text" placeholder="Search"> | |||
There was a problem hiding this comment.
Our placeholder conventions is Search…
There was a problem hiding this comment.
It helps the user distinguish between a placeholder label and a value.
There was a problem hiding this comment.
Will make those changes. Thanks for catching!
scss/component/_search.scss
Outdated
| width: fit-content; | ||
|
|
||
| .search-input { | ||
| background: url(../../images/icon/icon-20-o-search.svg) no-repeat 7px 7px; |
There was a problem hiding this comment.
We should try keep to multiples of 4, even for background-position.
Why not use 50% for vertical alignment?
There was a problem hiding this comment.
When i looked at the source code, it showed 7px. Will make a fix.
I dont believe I was using the px value to vertical align the icon, more so get the correct size what I saw in the inspector tool
|
@riichb nice work here. I left some feedback and leave @haseebjkhan for final review. |
|
Lastly, we should show an example of the radial gauge that isn't empty, maybe 50% full/complete. |
…eedback and change px size to be multiples of 4
There was a problem hiding this comment.
Great work 👍
Just agree with @karstenrowe about adding a filled state for the gauge. Will do the job.
What this does
Why did I do it?
Todo before merge
Follow up tasks
After