-
Notifications
You must be signed in to change notification settings - Fork 286
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
UI Tweaks April 2018 #780
UI Tweaks April 2018 #780
Conversation
Match BEM syntax in other parts of the app
I chose grays that in the middle of the lighter UI in Firefox and the darker UI in Chrome
- More consistent BEM CSS syntax - Whitespace here and there - Text and icon alignment
Thanks project Clarity: https://vmware.github.io/clarity These icons are consistent in weight and style
- Increase height - Consistent space around items - More clickable space around items - more visible selected item for toolbar radios
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.
Thanks so much for this PR! The content looks great. I think my only requests are that we try to abstract some things out.
I think we can use a common clear action / maybe make a component for it, and use svg-jar for the svgs, and make them actual svg files. Please let me know your thoughts and thanks for all the hard work!
app/controllers/view-tree.js
Outdated
@@ -38,6 +38,10 @@ export default Controller.extend({ | |||
})), | |||
|
|||
actions: { | |||
clearSearchText() { |
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 seems like we have a lot of different actions for clearing search. Could we perhaps name them all the same and maybe even have a base controller that has some common stuff to extend from?
<circle cx="5.5" cy="5.5" r="5.5"></circle> | ||
<path d="M1.98253524,1.98253524 L9,9" id="Line" stroke-linecap="square"></path> | ||
</g> | ||
<svg xmlns="http://www.w3.org/2000/svg" width="13" height="13"> |
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.
Would it perhaps be better to split this out into its own svg file, and then use it in the component? Perhaps with https://github.com/ivanvotti/ember-svg-jar?
@@ -0,0 +1,3 @@ | |||
<svg version="1.1" width="14" height="14" viewBox="0 0 36 36" preserveAspectRatio="xMidYMid meet" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> |
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.
Same about svgs here
@@ -4,5 +4,7 @@ | |||
<option value={{iframe.val}}>{{iframe.name}}</option> | |||
{{/each}} | |||
</select> | |||
<div class="dropdown__arrow"></div> | |||
<svg class="dropdown__arrow" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24.2 40.4"> |
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.
Same about svgs here
<g stroke="none" stroke-width="1" fill="none" fill-rule="evenodd"> | ||
<polygon class="svg-fill" fill="#000000" transform="translate(4.500000, 4.500000) rotate(-90.000000) translate(-4.500000, -4.500000) " points="4.5 0 9 9 0 9 "></polygon> | ||
</g> | ||
<svg width="21" height="21" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"> |
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.
Same about svgs here
app/templates/nav.hbs
Outdated
Data | ||
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px" width="19px" height="19px" viewBox="0 0 19 19" enable-background="new 0 0 19 19" xml:space="preserve"> | ||
<path d="M9.5,0.001C3.907,0.001,0,1.507,0,3.663v11.675C0,17.494,3.907,19,9.5,19c5.594,0,9.5-1.506,9.5-3.662V3.663 C19,1.507,15.094,0.001,9.5,0.001z M9.5,5.669c-4.768,0-7.81-1.318-7.81-2.007c0-0.689,3.042-2.008,7.81-2.008 c4.769,0,7.81,1.318,7.81,2.008C17.31,4.352,14.269,5.669,9.5,5.669z M17.31,15.338c0,0.689-3.041,2.007-7.81,2.007 c-4.768,0-7.81-1.317-7.81-2.007V5.852C3.39,6.77,6.282,7.324,9.5,7.324c3.217,0,6.108-0.554,7.81-1.472V15.338z"/> | ||
<svg aria-hidden="true" version="1.1" width="20" height="20" viewBox="0 0 36 36" preserveAspectRatio="xMidYMid meet" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> |
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.
Same about svgs here
app/templates/nav.hbs
Outdated
c-0.552,0.01-1.015-0.45-1.013-1.008c0.001-0.552,0.449-1.004,1-1.007C10.829,13.531,11.281,13.983,11.285,14.534z"/> | ||
</g> | ||
</g> | ||
<svg aria-hidden="true" version="1.1" width="20" height="20" viewBox="0 0 36 36" preserveAspectRatio="xMidYMid meet" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> |
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.
Same about svgs here
app/templates/nav.hbs
Outdated
<rect id="svg_3" height="6.815" width="3.33" fill="#454545" y="7.8805" x="7.737"/> | ||
<circle id="svg_4" r="1.753" cy="5.3775" cx="9.451" fill="#454545"/> | ||
<path id="svg_6" d="m9.5,19c-5.238,0 -9.5,-4.262 -9.5,-9.5c0,-5.238 4.262,-9.5 9.5,-9.5s9.5,4.262 9.5,9.5c0,5.238 -4.262,9.5 -9.5,9.5zm0,-17.434c-4.375,0 -7.933,3.559 -7.933,7.933c0,4.374 3.559,7.932 7.933,7.932c4.374,0 7.933,-3.559 7.933,-7.932c0,-4.374 -3.559,-7.933 -7.933,-7.933z" fill="#454545"/> | ||
<svg aria-hidden="true" version="1.1" width="20" height="20" viewBox="0 0 36 36" preserveAspectRatio="xMidYMid meet" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> |
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.
Same about svgs here
app/templates/nav.hbs
Outdated
</g> | ||
</g> | ||
</svg> | ||
<svg aria-hidden="true" version="1.1" width="20" height="20" viewBox="0 0 36 36" preserveAspectRatio="xMidYMid meet" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> |
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.
Same about svgs here
<div class="toolbar__search js-deprecations-search"> | ||
{{input value=searchVal placeholder="Search"}} | ||
{{#if searchVal}} | ||
<button {{action "clearSearchVal"}} class="toolbar__icon-button toolbar__search-clear-button" title="clear"> |
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 might also be good to put these common clear elements in some common template and use them, rather than repeating ourselves.
- Common partial for search fields - Same property name for text
@rwwagner90, change requests addressed. 👍 |
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.
Thanks for making those changes! svg-jar stuff is awesome and the search stuff is closer, but left a comment on it. I think we can simplify it in a component.
app/controllers/container-type.js
Outdated
@@ -18,6 +18,10 @@ export default Controller.extend({ | |||
}).property('[email protected]', 'search'), | |||
|
|||
actions: { | |||
clearSearch() { | |||
this.set('searchValue', ''); |
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.
We're closer now, with these all having the same name, but it would be awesome to just move this stuff to a common component, instead of defining the same action a bunch of times. Could we pull this stuff into a common component, and use that component instead of partials? I believe partials are deprecated or at least recommended against.
<circle id="svg_4" r="1.753" cy="5.3775" cx="9.451" fill="#454545"/> | ||
<path id="svg_6" d="m9.5,19c-5.238,0 -9.5,-4.262 -9.5,-9.5c0,-5.238 4.262,-9.5 9.5,-9.5s9.5,4.262 9.5,9.5c0,5.238 -4.262,9.5 -9.5,9.5zm0,-17.434c-4.375,0 -7.933,3.559 -7.933,7.933c0,4.374 3.559,7.932 7.933,7.932c4.374,0 7.933,-3.559 7.933,-7.932c0,-4.374 -3.559,-7.933 -7.933,-7.933z" fill="#454545"/> | ||
</svg> | ||
{{svg-jar "nav-info" width="20px" height="20px"}} |
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.
Love this svg-jar stuff! 🎉
@rwwagner90 |
Thanks @nummi! This looks great! |
service detection and cp code catching better service detection safer service detection cleanup template try catch for code getter to handle serialization issue UI Tweaks April 2018 (emberjs#780) * Pill CSS cleanup Match BEM syntax in other parts of the app * Lighten grays on background and borders I chose grays that in the middle of the lighter UI in Firefox and the darker UI in Chrome * High resolution Fishy Tomster image * Mixin sidebar cleanup - More consistent BEM CSS syntax - Whitespace here and there - Text and icon alignment * New Navigation icons Thanks project Clarity: https://vmware.github.io/clarity These icons are consistent in weight and style * Button for clearing search inputs * Toolbar tweaks - Increase height - Consistent space around items - More clickable space around items - more visible selected item for toolbar radios * Search consistification - Common partial for search fields - Same property name for text * Bring in svg-jar add-on * Search-field component Merge branch 'master' into computed-dependent-keys code to string update from comments code cleanup style cleanup code cleanup hasDependent keys computed template code cleanup icon sizing update Merge pull request #2 from lifeart/pr/781 icon sizing update attempt to fix tests style updates
* Pill CSS cleanup Match BEM syntax in other parts of the app * Lighten grays on background and borders I chose grays that in the middle of the lighter UI in Firefox and the darker UI in Chrome * High resolution Fishy Tomster image * Mixin sidebar cleanup - More consistent BEM CSS syntax - Whitespace here and there - Text and icon alignment * New Navigation icons Thanks project Clarity: https://vmware.github.io/clarity These icons are consistent in weight and style * Button for clearing search inputs * Toolbar tweaks - Increase height - Consistent space around items - More clickable space around items - more visible selected item for toolbar radios * Search consistification - Common partial for search fields - Same property name for text * Bring in svg-jar add-on * Search-field component
See corresponding Issue: #778
If reviewing I suggest looking at each commit individually.