Add some missing styles #207
Conversation
|
I'm closing this for now b/c I just realized it doesn't quite render correctly still. |
| $frost-color-nav-route-focus-box-shadow: #fff !default; | ||
|
|
||
| $frost-navigation-bar-height: 50px !default; | ||
| $z-index-application-bar: 3 !default; |
There was a problem hiding this comment.
FYI I've been trying to centralize all z-index values in core so that we know how they'll compete https://github.com/ciena-frost/ember-frost-core/blob/master/addon/styles/_z-index.scss
There was a problem hiding this comment.
Sounds good. I was just renaming existing variables and putting them in a variables file.
addon/styles/_variables.scss
Outdated
| $frost-color-nav-bar-bg: $frost-color-night-2 !default; | ||
| $frost-color-nav-border-left: rgba($frost-color-white, .2) !default; | ||
| $frost-color-nav-category-border-right: rgba($frost-color-white, .2) !default; | ||
| $frost-color-nav-category-active-bg: rgba(39, 36, 37, .5) !default; |
There was a problem hiding this comment.
We should have a TODO here to make this a colour variable if it isn't one already
There was a problem hiding this comment.
It is one, isn't it? Or do you mean add it to ember-frost-core?
addon/styles/_variables.scss
Outdated
| $frost-color-nav-category-border-right: rgba($frost-color-white, .2) !default; | ||
| $frost-color-nav-category-active-bg: rgba(39, 36, 37, .5) !default; | ||
| $frost-color-nav-column-border: rgba($frost-color-white, .3) !default; | ||
| $frost-color-nav-modal-bg: rgba(51, 66, 79, .97) !default; |
README.md
Outdated
| ### CSS | ||
| The following CSS classes are defined by this addon to make it easier to layout a page using `frost-navigation`: | ||
|
|
||
| * `frost-navigation-parent`: This class should be applied to, you guessed it, the parent of the `frost-navigation` |
There was a problem hiding this comment.
How about frost-application and frost-application-content?
| @@ -90,6 +91,7 @@ | |||
| "ember-computed-decorators": "^0.2.2", | |||
| "ember-cli-htmlbars": "^1.0.1", | |||
| "ember-cli-sass": "5.6.0", | |||
| "ember-frost-core": "^1.0.0", | |||
There was a problem hiding this comment.
Are we at the stage where core can work without a blueprint run? @dafortin
There was a problem hiding this comment.
I thought I put a note, maybe it wasn't in this PR, where the core install won't actually work, but I don't think it really matters for addons that depend on core. I don't think anyone is getting core by installing some other addon in their app. I think it's safe (for this transition period at least) to just tell people to install core and test independently.
There was a problem hiding this comment.
Yeah, that's fine - I doubt we have any scenarios where only the nav is used currently anyway
| }} | ||
|
|
||
| {{#frost-popover | ||
| class='frost-navigation-user-menu' |
There was a problem hiding this comment.
Never mind - this makes sense, but do you want the other CSS concats in this file?
There was a problem hiding this comment.
Yeah, I think so, because those aren't classes provided by this addon. Of course, maybe I should make the user menu an actual component that this guy provides? Doesn't mean people have to use it, so it won't break anyone, but might make life easier for some.
There was a problem hiding this comment.
Actually, maybe in another PR. I'm gonna try this one with just the style updates and the example user-menu this time, since I'm not sure the best way to allow consumers to define their own additional items in the user menu dropdown.
|
Changes Unknown when pulling 7f8d456 on job13er:add-some-styles into ** on ciena-frost:master**. |
|
@sglanzer Added screenshots for previous rendering and current rendering to PR description. |
|
@sglanzer How's this looking now? |
|
@sglanzer Ping |
|
Approved - thanks @job13er, sorry about the delay |
This project uses semver, please check the scope of this pr:
Screenshots
Before
After
CHANGELOG