-
Notifications
You must be signed in to change notification settings - Fork 9
Add LocalNav CSS component. #11
Changes from all commits
366dbf3
2c36f41
235a7dc
6824747
6c15ae7
c1e27ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
|
|
||
| $localNavSideSpacing: 10px; | ||
|
|
||
| @import "local_breadcrumbs"; | ||
| @import "local_dropdown"; | ||
| @import "local_menu"; | ||
| @import "local_nav"; | ||
| @import "local_search"; | ||
| @import "local_tabs"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
|
|
||
| /** | ||
| * 1. Breadcrumbs are placed in the top-left corner and need to be bumped over | ||
| * a bit. | ||
| */ | ||
| .localBreadcrumbs { | ||
| display: flex; | ||
| align-items: center; | ||
| height: 100%; | ||
| padding-left: $localNavSideSpacing; /* 1 */ | ||
| } | ||
|
|
||
| .localBreadcrumb { | ||
| & + & { | ||
| margin-left: 6px; | ||
|
|
||
| &:before { | ||
| content: '/'; | ||
| user-select: none; | ||
| margin-right: 4px; | ||
| color: #5a5a5a; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .localBreadcrumb__link { | ||
| font-size: 14px; | ||
| color: #5a5a5a; | ||
| text-decoration: none; | ||
|
|
||
| &:hover { | ||
| text-decoration: underline; | ||
| } | ||
| } | ||
|
|
||
| .localBreadcrumb__emphasis { | ||
| font-weight: 700; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
|
|
||
| .localDropdown { | ||
| padding: 0 $localNavSideSpacing; | ||
| background-color: #f6f6f6; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
|
|
||
| .localMenu { | ||
| display: flex; | ||
| align-items: center; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .localMenuItem { | ||
| display: flex; | ||
| align-items: center; | ||
| height: 100%; | ||
| padding: 0 $localNavSideSpacing; | ||
| font-size: 14px; | ||
| background-color: transparent; | ||
| color: #5a5a5a; | ||
| border: 0; | ||
| cursor: pointer; | ||
|
|
||
| &:hover { | ||
| background-color: rgba(#000000, 0.1); | ||
| color: #000000; | ||
| } | ||
|
|
||
| &.localMenuItem-is-selected { | ||
| background-color: #f6f6f6; | ||
| } | ||
| } | ||
|
|
||
| .localMenuItem__icon { | ||
| margin-right: 5px; | ||
| margin-bottom: -1px; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
|
|
||
| /** | ||
| * 1. Match height of logo in side bar, but allow it to expand to accommodate | ||
| * dropdown. | ||
| */ | ||
| .localNav { | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: space-between; | ||
| min-height: 70px; /* 1 */ | ||
| background-color: #e4e4e4; | ||
| } | ||
|
|
||
| .localNavRow { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| height: 32px; | ||
| } | ||
|
|
||
| .localNavRow__section { | ||
| height: 100%; | ||
| } | ||
|
|
||
| .localNavRow--secondary { | ||
| height: 38px; | ||
| padding: 0 $localNavSideSpacing; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
|
|
||
| $localSearchHeight: 30px; | ||
|
|
||
| .localSearch { | ||
| display: flex; | ||
| width: 100%; | ||
| height: $localSearchHeight; | ||
| } | ||
|
|
||
| .localSearch__input { | ||
| flex: 1 1 100%; | ||
| padding: 5px 15px; | ||
| font-size: 14px; | ||
| color: #2d2d2d; | ||
| border: 0; | ||
| border-bottom-left-radius: 4px; | ||
| border-top-left-radius: 4px; | ||
| border-bottom-right-radius: 0; | ||
| border-top-right-radius: 0; | ||
| } | ||
|
|
||
| .localSearch__button { | ||
| width: 43px; | ||
| height: $localSearchHeight; | ||
| font-size: 14px; | ||
| background-color: #9c9c9c; | ||
| color: #ffffff; | ||
| border: 0; | ||
| border-bottom-left-radius: 0; | ||
| border-top-left-radius: 0; | ||
| border-bottom-right-radius: 4px; | ||
| border-top-right-radius: 4px; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
|
|
||
| .localTabs { | ||
| display: flex; | ||
| align-items: center; | ||
| height: 100%; | ||
| } | ||
|
|
||
| /** | ||
| * 1. Make sure the bottom border is flush with the bottom of the LocalNav. | ||
| */ | ||
| .localTab { | ||
| padding: 5px 0 6px 0; | ||
| font-size: 18px; | ||
| line-height: 22px; /* 1 */ | ||
| color: #2d2d2d; | ||
| border-bottom: 2px solid transparent; | ||
| text-decoration: none; | ||
|
|
||
| &.localTab-is-selected { | ||
| border-bottom-color: #2d2d2d; | ||
| } | ||
|
|
||
| & + & { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, why is this any less baffling than
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it easier to read because it's a pattern I use all the time, so for me it mentally translates into "when this element is in a list...". The main concern I have with your suggestion is that a reader of these styles will have to compare the class name with the enclosing class name to make sure they're the same, whereas with How about we try it this way for a month and see if you can get used to it? If not then I will go back and change everything to the format you're suggesting. |
||
| margin-left: 8px; | ||
| } | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| @import "components/top_nav/index"; | ||
| @import "components/local_nav/index"; |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
|
|
||
| import { | ||
| GuideExample, | ||
| } from '../../components'; | ||
|
|
||
| export default function creatExample(examples) { | ||
| class Example extends GuideExample { | ||
| constructor(props) { | ||
| super(props, examples); | ||
| } | ||
| } | ||
|
|
||
| Example.propTypes = Object.assign({}, GuideExample.propTypes); | ||
|
|
||
| return Example; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels un-React-y. I would expect something along the lines of Here's an example from the Cloud UI styleguide: https://github.com/elastic/cloud/blob/master/cloud-ui/public/components/Styleguide/index.js#L108-L146 Ours is probably a simpler use-case than this though. I need to get some Cloud stuff done after some vacation, but I'll cycle back and try to get an overview of the approach. Maybe we could do a zoom?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, sounds good! Want to put something on the calendar? Brief explanation of how this evolved: I started out in the same place as the Cloud UI styleguide, but found every example needed to adhere to the same format: title, link to code viewer, text description, and then arbitrary HTML and JS interactive demo. So to wire this up manually would be a lot of boilerplate. GuideExample automates all of that work, but of course when you codify things like this you lose customizability. Technically, there's nothing stopping an engineer from manually duplicating the functionality of GuideExample and regaining full customizability, but my hypothesis is that would be a big time-sink for very little benefit. |
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Why not just
> .localBreadcrumb? This feels mind-boggling.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.
Well, the point is that it's a sibling selector. This could be replaced by un-nesting this selector and changing it to
.localBreadcrumb + .localBreadcrumbbut I think that de-centralizes the styles too much and will make it a little more difficult to maintain.