Skip to content

Section isolated mode #394

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

Merged
merged 4 commits into from
May 11, 2017
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
31 changes: 20 additions & 11 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import React from 'react';
import ReactDOM from 'react-dom';
import isFinite from 'lodash/isFinite';
import {
getComponentNameFromHash,
getInfoFromHash,
filterComponentExamples,
filterComponentsInSectionsByExactName,
filterSections,
processSections,
setSlugs,
slugger,
Expand All @@ -27,24 +28,31 @@ function renderStyleguide() {

// Parse URL hash to check if the components list must be filtered
const {
// Name of the filtered component to show isolated (/#!/Button → Button)
targetComponentName,
// Name of the filtered component/section to show isolated (/#!/Button → Button)
targetName,
// Index of the fenced block example of the filtered component isolate (/#!/Button/1 → 1)
targetComponentIndex,
} = getComponentNameFromHash();
targetIndex,
} = getInfoFromHash();

let isolatedComponent = false;
let isolatedExample = false;
let isolatedSection = false;

// Filter the requested component id required
if (targetComponentName) {
const filteredComponents = filterComponentsInSectionsByExactName(sections, targetComponentName);
sections = [{ components: filteredComponents }];
isolatedComponent = true;
if (targetName) {
const filteredComponents = filterComponentsInSectionsByExactName(sections, targetName);
if (filteredComponents.length) {
sections = [{ components: filteredComponents }];
isolatedComponent = true;
}
else {
sections = [filterSections(sections, targetName)];
isolatedSection = true;
}

// If a single component is filtered and a fenced block index is specified hide the other examples
if (filteredComponents.length === 1 && isFinite(targetComponentIndex)) {
filteredComponents[0] = filterComponentExamples(filteredComponents[0], targetComponentIndex);
if (filteredComponents.length === 1 && isFinite(targetIndex)) {
filteredComponents[0] = filterComponentExamples(filteredComponents[0], targetIndex);
isolatedExample = true;
}
}
Expand All @@ -62,6 +70,7 @@ function renderStyleguide() {
sections={sections}
isolatedComponent={isolatedComponent}
isolatedExample={isolatedExample}
isolatedSection={isolatedSection}
/>,
document.getElementById('app')
);
Expand Down
8 changes: 7 additions & 1 deletion src/rsg-components/Section/Section.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Components from 'rsg-components/Components';
import Sections from 'rsg-components/Sections';
import SectionRenderer from 'rsg-components/Section/SectionRenderer';

export default function Section({ section }) {
export default function Section({ section }, { isolatedSection = false }) {
const { name, slug, content, components, sections } = section;

const contentJsx = content && (
Expand All @@ -21,17 +21,23 @@ export default function Section({ section }) {
sections={sections}
/>
);

return (
<SectionRenderer
name={name}
slug={slug}
content={contentJsx}
components={componentsJsx}
sections={sectionsJsx}
isolatedSection={isolatedSection}
/>
);
}

Section.propTypes = {
section: PropTypes.object.isRequired,
};

Section.contextTypes = {
isolatedSection: PropTypes.bool,
};
37 changes: 33 additions & 4 deletions src/rsg-components/Section/SectionRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,52 @@ import React from 'react';
import PropTypes from 'prop-types';
import Styled from 'rsg-components/Styled';
import Heading from 'rsg-components/Heading';
import Link from 'rsg-components/Link';

const styles = ({ space, fontFamily, fontSize }) => ({
root: {
marginBottom: space[4],
'&:hover $isolatedLink': {
isolate: false,
opacity: 1,
},
},
heading: {
margin: [[0, 0, space[2]]],
fontFamily: fontFamily.base,
fontSize: fontSize.h1,
},
isolatedLink: {
position: 'absolute',
top: 0,
right: 0,
fontFamily: fontFamily.base,
fontSize: fontSize.base,
opacity: 0,
transition: 'opacity ease-in-out .15s .2s',
},
titleWrapper: {
position: 'relative',
},
});

export function SectionRenderer({ classes, name, slug, content, components, sections }) {
export function SectionRenderer({ classes, name, slug, content, components, sections, isolatedSection }) {
return (
<section className={classes.root}>
{name && (
<Heading level={1} slug={slug} className={classes.heading}>{name}</Heading>
)}
<div className={classes.titleWrapper}>
{name && (
<Heading level={1} slug={slug} className={classes.heading}>{name}</Heading>
)}
<div className={classes.isolatedLink}>
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it’s time to extract it to a separate component. We already have this pattern in two places I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created an IsolatedLink component that contains the markup and the CSS for it. I believe that the CSS rule that says "on hover of this element, reveal the IsolatedLink" should be inside the component (the component would just need the className for the element that will reveal the component once that element is hovered).

Do you agree with this approach?

Trying to implement this, I run into a problem with how to use this class inside the styles object. Following is the IsolateLinkRender where hoverClass is the class I'm getting from the parent component, and trying to use it in the styles object. Obviously the following doesn't work, since I don't know how to pass the hoverLink from the component to the styles object. Any help?

const styles = ({ font }) => ({
	[hoverClass]: {
		'&:hover $isolatedLink': {
			isolate: false,
			opacity: 1,
		},
	},
	isolatedLink: {
		position: 'absolute',
		top: 0,
		right: 0,
		fontFamily: font,
		fontSize: 14,
		opacity: 0,
		transition: 'opacity ease-in-out .15s .2s',
	},
});

export function IsolatedLinkRenderer({ classes, hoverClass, isIsolated, link }) {
	return (
		<div className={classes.isolatedLink}>
			{link && (
				isIsolated ? (
					<Link href="/">⇽ Back</Link>
				) : (
					<Link href={'#!/' + link}>Open isolated ⇢</Link>
				)
			)}
		</div>
	);
}

IsolatedLinkRenderer.propTypes = {
	classes: PropTypes.object.isRequired,
	hoverClass: PropTypes.string.isRequired,
	isIsolated: PropTypes.bool,
	link: PropTypes.string,
};

export default Styled(styles)(IsolatedLinkRenderer);

Copy link
Member

Choose a reason for hiding this comment

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

I believe that the CSS rule that says "on hover of this element, reveal the IsolatedLink" should be inside the component

With this I agree.

the component would just need the className for the element that will reveal the component once that element is hovered

I think you need to put both of them into this component. Sou it’d be not IsolatedLink but more like IsolatedContainer:

<IsolatedContainer to="/foo">
  this content will have a link in the corner
</IsolatedContainer>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sapegin I created the IsolatedContainer component. Since the CSS rule for "onHover" is inside the component, that means that on heading hover show the "Open Isolated" link (it used to be onHover of the whole component info area show the link). That seems fine by me, but theres a problem with the Isolated example which has no title. It used to be on hover of the code example area but now the only "hover" area is the link itself which is hidden.

Any ideas? Do you want me to update the PR to see the code?

Copy link
Member

Choose a reason for hiding this comment

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

Yes please!

{name && (
isolatedSection ? (
<Link href="/">⇽ Back</Link>
Copy link
Member

Choose a reason for hiding this comment

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

And a test for back link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to understand how to add tests for links, but I couldn't figure it out. Do you mind adding it?

Copy link
Member

Choose a reason for hiding this comment

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

That’s fine, we’re working on a new UI for these links anyway ;-)

) : (
<Link href={'#!/' + name}>Open isolated ⇢</Link>
)
)}
</div>
</div>
{content}
{components}
{sections}
Expand All @@ -34,6 +62,7 @@ SectionRenderer.propTypes = {
content: PropTypes.node,
components: PropTypes.node,
sections: PropTypes.node,
isolatedSection: PropTypes.bool,
};

export default Styled(styles)(SectionRenderer);
55 changes: 42 additions & 13 deletions src/rsg-components/Section/__snapshots__/Section.spec.js.snap
Original file line number Diff line number Diff line change
@@ -1,15 +1,30 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`render should not render title if name is not set 1`] = `<section />`;
exports[`render should not render title if name is not set 1`] = `
<section>
Copy link
Member

Choose a reason for hiding this comment

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

These empty divs look strange but they most likely will go away after UI 2.0 ;-)

<div>
<div />
</div>
</section>
`;

exports[`render should render component 1`] = `
<section>
<_class
level={1}
slug="foo"
>
Foo
</_class>
<div>
<_class
level={1}
slug="foo"
>
Foo
</_class>
<div>
<_class
href="#!/Foo"
>
Open isolated ⇢
</_class>
</div>
</div>
<Examples
examples={
Array [
Expand All @@ -36,24 +51,35 @@ exports[`render should render component 1`] = `

exports[`render should render title if name is set 1`] = `
<section>
<_class
level={1}
slug="test"
>
test
</_class>
<div>
<_class
level={1}
slug="test"
>
test
</_class>
<div>
<_class
href="#!/test"
>
Open isolated ⇢
</_class>
</div>
</div>
</section>
`;

exports[`should not render components list if not defined 1`] = `
<_class
isolatedSection={false}
name="No components"
slug="no-components"
/>
`;

exports[`should not render sections if not defined 1`] = `
<_class
isolatedSection={false}
name="No sections"
slug="no-sections"
/>
Expand Down Expand Up @@ -83,6 +109,7 @@ exports[`should render component renderer 1`] = `
}
/>
}
isolatedSection={false}
name="Foo"
sections={
<Sections
Expand All @@ -100,13 +127,15 @@ exports[`should render components list 1`] = `
components={Array []}
/>
}
isolatedSection={false}
name="Components"
slug="components"
/>
`;

exports[`should render sections if defined 1`] = `
<_class
isolatedSection={false}
name="Nested sections"
sections={
<Sections
Expand Down
3 changes: 3 additions & 0 deletions src/rsg-components/StyleGuide/StyleGuide.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ export default class StyleGuide extends Component {
patterns: PropTypes.array,
isolatedComponent: PropTypes.bool,
isolatedExample: PropTypes.bool,
isolatedSection: PropTypes.bool,
};

static childContextTypes = {
codeKey: PropTypes.number.isRequired,
config: PropTypes.object.isRequired,
isolatedComponent: PropTypes.bool,
isolatedExample: PropTypes.bool,
isolatedSection: PropTypes.bool,
};

static defaultProps = {
Expand All @@ -34,6 +36,7 @@ export default class StyleGuide extends Component {
config: this.props.config,
isolatedComponent: this.props.isolatedComponent,
isolatedExample: this.props.isolatedExample,
isolatedSection: this.props.isolatedSection,
};
}

Expand Down
35 changes: 31 additions & 4 deletions src/utils/__tests__/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,18 +243,45 @@ describe('filterSectionsByName', () => {
});
});

describe('getComponentNameFromHash', () => {
describe('getInfoFromHash', () => {
it('should return important part of hash if it contains component name', () => {
const result = utils.getComponentNameFromHash('#!/Button');
expect(result).toEqual({ targetComponentName: 'Button', targetComponentIndex: null });
const result = utils.getInfoFromHash('#!/Button');
expect(result).toEqual({ targetName: 'Button', targetIndex: null });
});

it('should return an empty object if hash contains no component name', () => {
const result = utils.getComponentNameFromHash('Button');
const result = utils.getInfoFromHash('Button');
expect(result).toEqual({});
});
});

describe('filterSections', () => {
const sections = [
{
name: 'General',
sections: [],
},
{
name: 'Forms',
sections: [],
},
{
name: 'Lists',
sections: [],
},
];

it('should return the Forms section', () => {
const result = utils.filterSections(sections, 'Forms');
expect(result).toEqual(sections[1]);
});

it('should return the Lists section', () => {
const result = utils.filterSections(sections, 'Lists');
expect(result).toEqual(sections[2]);
});
});

describe('filterComponentExamples', () => {
it('should return a shallow copy of the component with example filtered by given index', () => {
const comp = {
Expand Down
22 changes: 16 additions & 6 deletions src/utils/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,31 @@ export function filterComponentsInSectionsByExactName(sections, name) {
}

/**
* Returns an object containing component name and, optionally, an example index
* Filters the sections to find the one with the matching name
* @param {Array} sections The styleguide sections
* @param {string} name The name to match
* @return {object} The section found
*/
export function filterSections(sections, name) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test for this function too? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tests for this function.

return sections.find(section => section.name === name);
}

/**
* Returns an object containing component/section name and, optionally, an example index
* from hash part or page URL:
* http://localhost:6060/#!/Button → { targetComponentName: 'Button' }
* http://localhost:6060/#!/Button/1 → { targetComponentName: 'Button', targetComponentIndex: 1 }
* http://localhost:6060/#!/Button → { targetName: 'Button' }
* http://localhost:6060/#!/Button/1 → { targetName: 'Button', targetIndex: 1 }
*
* @param {string} [hash]
* @returns {object}
*/
export function getComponentNameFromHash(hash = window.location.hash) {
export function getInfoFromHash(hash = window.location.hash) {
if (hash.substr(0, 3) === '#!/') {
const tokens = hash.substr(3).split('/');
const index = parseInt(tokens[1], 10);
return {
targetComponentName: tokens[0],
targetComponentIndex: isNaN(index) ? null : index,
targetName: tokens[0],
targetIndex: isNaN(index) ? null : index,
};
}
return {};
Expand Down