Skip to content
This repository was archived by the owner on Feb 7, 2018. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/framework/components/local_nav/_index.scss
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";
38 changes: 38 additions & 0 deletions src/framework/components/local_nav/_local_breadcrumbs.scss
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 {
& + & {
Copy link
Copy Markdown

@bevacqua bevacqua Jul 25, 2016

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.

Copy link
Copy Markdown
Contributor Author

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 + .localBreadcrumb but I think that de-centralizes the styles too much and will make it a little more difficult to maintain.

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;
}
5 changes: 5 additions & 0 deletions src/framework/components/local_nav/_local_dropdown.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

.localDropdown {
padding: 0 $localNavSideSpacing;
background-color: #f6f6f6;
}
32 changes: 32 additions & 0 deletions src/framework/components/local_nav/_local_menu.scss
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;
}
28 changes: 28 additions & 0 deletions src/framework/components/local_nav/_local_nav.scss
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;
}
33 changes: 33 additions & 0 deletions src/framework/components/local_nav/_local_search.scss
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;
}
26 changes: 26 additions & 0 deletions src/framework/components/local_nav/_local_tabs.scss
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;
}

& + & {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, why is this any less baffling than + .localTab?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 & + & there's no doubt.

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;
}
}
12 changes: 0 additions & 12 deletions src/framework/components/top_nav/_index.scss

This file was deleted.

2 changes: 1 addition & 1 deletion src/framework/framework.scss
Original file line number Diff line number Diff line change
@@ -1 +1 @@
@import "components/top_nav/index";
@import "components/local_nav/index";
1 change: 1 addition & 0 deletions src/guide/components/guide_example/guide_example.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ GuideExample.propTypes = {
route: PropTypes.object.isRequired,
sections: PropTypes.arrayOf(React.PropTypes.shape({
title: React.PropTypes.string.isRequired,
description: React.PropTypes.any,
html: React.PropTypes.string.isRequired,
js: React.PropTypes.string,
})),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,12 @@
inset 0 2px 8px rgba(black, 0.2);
}
}

.guidePageSection__description {
font-size: 14px;
line-height: 21px;
}

.guidePageSection__example--standalone {
margin-top: 10px;
}
40 changes: 32 additions & 8 deletions src/guide/components/guide_page_section/guide_page_section.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ import React, {
PropTypes,
} from 'react';

import classNames from 'classnames';

import {
JsInjector,
} from '../../services';

import {
Html,
} from '../';

export default class GuidePageSection extends Component {

constructor(props) {
Expand All @@ -38,6 +36,16 @@ export default class GuidePageSection extends Component {
// the component DOM is available for the JS to manipulate.
JsInjector.inject(this.props.js, this.scriptId);
}

function trimChildren(node) {
if (node.children.length > 0) {
[...node.children].forEach(trimChildren);
return;
}
node.textContent = node.textContent.trim();
}

trimChildren(this.refs.html);
}

componentWillUnmount() {
Expand All @@ -50,6 +58,20 @@ export default class GuidePageSection extends Component {
}

render() {
let description;

if (this.props.children) {
description = (
<div className="guidePageSection__description">
{this.props.children}
</div>
);
}

const exampleClasses = classNames({
'guidePageSection__example--standalone': !this.props.children,
});

return (
<div
id={this.props.slug}
Expand All @@ -65,11 +87,13 @@ export default class GuidePageSection extends Component {
/>
</div>

<div>
{this.props.children}
</div>
{description}

<Html>{this.props.html}</Html>
<div
ref="html"
className={exampleClasses}
dangerouslySetInnerHTML={{ __html: this.props.html }}
/>
</div>
);
}
Expand Down
23 changes: 0 additions & 23 deletions src/guide/components/html/Html.jsx

This file was deleted.

3 changes: 0 additions & 3 deletions src/guide/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,3 @@ export { default as GuidePageSideNav } from './guide_page_side_nav/guide_page_si

export * from './guide_page_side_nav/guide_page_side_nav_item.jsx';
export { default as GuidePageSideNavItem } from './guide_page_side_nav/guide_page_side_nav_item.jsx';

export * from './html/html.jsx';
export { default as Html } from './html/html.jsx';
16 changes: 16 additions & 0 deletions src/guide/services/example/createExample.js
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;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels un-React-y. I would expect something along the lines of <Examples examples={ examples } />. Or that we'd just use props for this. Might be something I'm missing though — haven't had time to look at this repo yet.

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
And the Example component: https://github.com/elastic/cloud/blob/master/cloud-ui/public/components/Styleguide/index.js#L30-L41

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

3 changes: 3 additions & 0 deletions src/guide/services/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@

export * from './example/createExample';
export { default as createExample } from './example/createExample';

export * from './js_injector/js_injector';
export { default as JsInjector } from './js_injector/js_injector';

Expand Down
8 changes: 4 additions & 4 deletions src/guide/services/routes/Routes.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@

import Slugify from '../string/slugify';

import TopNavExample
from '../../views/top_nav/top_nav_example.jsx';
import LocalNavExample
from '../../views/local_nav/local_nav_example.jsx';

// Component route names should match the component name exactly.
const components = [{
name: 'TopNav',
component: TopNavExample,
name: 'LocalNav',
component: LocalNavExample,
}];

export default {
Expand Down
2 changes: 1 addition & 1 deletion src/guide/variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ $guideNavHeight: 60px;
$guideSideNavWidth: 400px;
$guideSideNavSmallWidth: 220px;
$guideCodeViewerWidth: 700px;
$guideCodeViewerSmallWidth: 400px;
$guideCodeViewerSmallWidth: 580px;
$guideCodeViewerTransition: 0.2s ease;

$normalBreakpoint: 1900px;
Expand Down
Loading