Repository: styleguidist/react-styleguidist
Pull request #1304 (2019-03-04 04:30:03)

Add more Cypress tests for isolated mode toggling
Some work towards https://github.com/styleguidist/react-styleguidist/issues/776

codecov-io: # [Codecov](https://codecov.io/gh/styleguidist/react-styleguidist/pull/1304?src=pr&el=h1) Report
> Merging [#1304](https://codecov.io/gh/styleguidist/react-styleguidist/pull/1304?src=pr&el=desc) into [master](https://codecov.io/gh/styleguidist/react-styleguidist/commit/1abe4c2a067f15040586e135b24e6deb6b82e97c?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

| [Impacted Files](https://codecov.io/gh/styleguidist/react-styleguidist/pull/1304?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/scripts/utils/sanitizeConfig.js](https://codecov.io/gh/styleguidist/react-styleguidist/pull/1304/diff?src=pr&el=tree#diff-c3JjL3NjcmlwdHMvdXRpbHMvc2FuaXRpemVDb25maWcuanM=) | `100% <0%> (ø)` | :arrow_up: |
| [src/scripts/utils/error.js](https://codecov.io/gh/styleguidist/react-styleguidist/pull/1304/diff?src=pr&el=tree#diff-c3JjL3NjcmlwdHMvdXRpbHMvZXJyb3IuanM=) | `100% <0%> (ø)` | :arrow_up: |
| [src/scripts/config.js](https://codecov.io/gh/styleguidist/react-styleguidist/pull/1304/diff?src=pr&el=tree#diff-c3JjL3NjcmlwdHMvY29uZmlnLmpz) | `94.11% <0%> (ø)` | :arrow_up: |

dougmacknz: Alright, I've replaced out some of the other selectors with new test ids as well, hope you don't mind.

A couple of the decisions I made, with the reasoning, let me know if there's anything you disagree with:
- I've used `data-testid` rather than `data-cy`
    - It's library agnostic, in case we want to use it with another library in the future.
    - It's quick and easy to understand its purpose (some people might not know what `cy` is)
    - It's one of the recommended names by Cypress and also used in https://github.com/kentcdodds/dom-testing-library FWIW
* I've made `testid`'s unique (except for my last commit where I don't have some data needed to make it unique) eg. `data-testid="Button-examples"` & `data-testid="CounterButton-examples"` rather than a generic `data-testid="component-examples"`
    - I didn't see any documentation around best practice for this, but I'm guessing since it's called an ID it should probably be unique.
    - We may want to target a particular element in the future- in that case it would be best to have it unique from the start.
    - On the downside this does make selecting a bit more flaky (eg `[data-testid$="-container"]`) rather than being able to use the exact string.

Since you last saw this PR I've also combined tests/assertions that relied on state from the previous test into a single test. This was after reading further down on the Best Practices document - https://docs.cypress.io/guides/references/best-practices.html#Having-tests-rely-on-the-state-of-previous-tests
stale[bot]: 😴 This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week without any further activity. Consider opening a pull request if you still have this issue or want this feature.

dougmacknz: @sapegin would you like me to rebase this and resolve the conflicts?
sapegin: This would be great!
dougmacknz: Rebased!
dougmacknz: Have fixed those up now :)

Patch:
diff --git a/examples/styled-components/babel.config.js b/examples/styled-components/babel.config.js
index fd680b6a..9838fe5a 100644
--- a/examples/styled-components/babel.config.js
+++ b/examples/styled-components/babel.config.js
@@ -5,11 +5,11 @@ module.exports = {
 			{
 				modules: false,
 				useBuiltIns: 'usage',
-				corejs: 3
+				corejs: 3,
 			},
 		],
 		['@babel/preset-react'],
-		['@babel/preset-typescript']
+		['@babel/preset-typescript'],
 	],
 	plugins: ['@babel/plugin-proposal-class-properties', 'babel-plugin-styled-components'],
 };
diff --git a/src/client/rsg-components/Examples/Examples.js b/src/client/rsg-components/Examples/Examples.js
index c15663de..d7c5d0a5 100644
--- a/src/client/rsg-components/Examples/Examples.js
+++ b/src/client/rsg-components/Examples/Examples.js
@@ -6,7 +6,7 @@ import ExamplesRenderer from 'rsg-components/Examples/ExamplesRenderer';
 
 export default function Examples({ examples, name, exampleMode }, { codeRevision }) {
 	return (
-		<ExamplesRenderer>
+		<ExamplesRenderer name={name}>
 			{examples.map((example, index) => {
 				switch (example.type) {
 					case 'code':
diff --git a/src/client/rsg-components/Examples/ExamplesRenderer.js b/src/client/rsg-components/Examples/ExamplesRenderer.js
index 1f44adc7..44301610 100644
--- a/src/client/rsg-components/Examples/ExamplesRenderer.js
+++ b/src/client/rsg-components/Examples/ExamplesRenderer.js
@@ -7,12 +7,17 @@ const styles = () => ({
 	root: {},
 });
 
-export function ExamplesRenderer({ classes, children }) {
-	return <article className={classes.root}>{children}</article>;
+export function ExamplesRenderer({ classes, name, children }) {
+	return (
+		<article className={classes.root} data-testid={`${name}-examples`}>
+			{children}
+		</article>
+	);
 }
 
 ExamplesRenderer.propTypes = {
 	classes: PropTypes.object.isRequired,
+	name: PropTypes.string.isRequired,
 	children: PropTypes.node,
 };
 
diff --git a/src/client/rsg-components/Examples/__snapshots__/Examples.spec.js.snap b/src/client/rsg-components/Examples/__snapshots__/Examples.spec.js.snap
index 0beff09c..9b53a5f4 100644
--- a/src/client/rsg-components/Examples/__snapshots__/Examples.spec.js.snap
+++ b/src/client/rsg-components/Examples/__snapshots__/Examples.spec.js.snap
@@ -1,7 +1,9 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`should render examples 1`] = `
-<Styled(Examples)>
+<Styled(Examples)
+  name="button"
+>
   <Playground
     code=
 <button>
diff --git a/src/client/rsg-components/Playground/Playground.js b/src/client/rsg-components/Playground/Playground.js
index c31b3d72..513249e8 100644
--- a/src/client/rsg-components/Playground/Playground.js
+++ b/src/client/rsg-components/Playground/Playground.js
@@ -82,6 +82,7 @@ class Playground extends Component {
 		) : (
 			<PlaygroundRenderer
 				name={name}
+				exampleIndex={index}
 				preview={preview}
 				previewProps={settings.props || {}}
 				tabButtons={
diff --git a/src/client/rsg-components/Playground/Playground.spec.js b/src/client/rsg-components/Playground/Playground.spec.js
index fd81fc38..588966d1 100644
--- a/src/client/rsg-components/Playground/Playground.spec.js
+++ b/src/client/rsg-components/Playground/Playground.spec.js
@@ -127,6 +127,7 @@ it('renderer should render preview', () => {
 	const actual = shallow(
 		<PlaygroundRenderer
 			classes={classes(styles)}
+			exampleIndex={0}
 			name="name"
 			preview={<div>preview</div>}
 			previewProps={{ className: 'pizza', title: 'salami' }}
diff --git a/src/client/rsg-components/Playground/PlaygroundRenderer.js b/src/client/rsg-components/Playground/PlaygroundRenderer.js
index cd668ab8..401e8463 100644
--- a/src/client/rsg-components/Playground/PlaygroundRenderer.js
+++ b/src/client/rsg-components/Playground/PlaygroundRenderer.js
@@ -28,6 +28,7 @@ export const styles = ({ space, color, borderRadius }) => ({
 
 export function PlaygroundRenderer({
 	classes,
+	exampleIndex,
 	name,
 	preview,
 	previewProps,
@@ -37,7 +38,7 @@ export function PlaygroundRenderer({
 }) {
 	const { className, ...props } = previewProps;
 	return (
-		<div className={classes.root}>
+		<div className={classes.root} data-testid={`${name}-example-${exampleIndex}`}>
 			<div className={cx(classes.preview, className)} {...props} data-preview={name}>
 				{preview}
 			</div>
@@ -52,6 +53,7 @@ export function PlaygroundRenderer({
 
 PlaygroundRenderer.propTypes = {
 	classes: PropTypes.object.isRequired,
+	exampleIndex: PropTypes.number.isRequired,
 	name: PropTypes.string.isRequired,
 	preview: PropTypes.node.isRequired,
 	previewProps: PropTypes.object.isRequired,
diff --git a/src/client/rsg-components/Playground/__snapshots__/Playground.spec.js.snap b/src/client/rsg-components/Playground/__snapshots__/Playground.spec.js.snap
index 038868b5..1bbdf10a 100644
--- a/src/client/rsg-components/Playground/__snapshots__/Playground.spec.js.snap
+++ b/src/client/rsg-components/Playground/__snapshots__/Playground.spec.js.snap
@@ -3,6 +3,7 @@
 exports[`renderer should render preview 1`] = `
 <div
   className="root"
+  data-testid="name-example-0"
 >
   <div
     className="preview pizza"
@@ -41,6 +42,7 @@ exports[`renderer should render preview 1`] = `
 
 exports[`should render component renderer 1`] = `
 <Styled(Playground)
+  exampleIndex={0}
   name="name"
   preview={
     <Preview
diff --git a/src/client/rsg-components/ReactComponent/ReactComponentRenderer.js b/src/client/rsg-components/ReactComponent/ReactComponentRenderer.js
index faa3a90f..dfbc9aa0 100644
--- a/src/client/rsg-components/ReactComponent/ReactComponentRenderer.js
+++ b/src/client/rsg-components/ReactComponent/ReactComponentRenderer.js
@@ -39,7 +39,7 @@ export function ReactComponentRenderer({
 	tabBody,
 }) {
 	return (
-		<div className={classes.root} id={name + '-container'}>
+		<div className={classes.root} data-testid={`${name}-container`}>
 			<header className={classes.header}>
 				{heading}
 				{pathLine && <Pathline>{pathLine}</Pathline>}
diff --git a/src/client/rsg-components/ReactComponent/__snapshots__/ReactComponent.spec.js.snap b/src/client/rsg-components/ReactComponent/__snapshots__/ReactComponent.spec.js.snap
index 603a296d..3e7b4fde 100644
--- a/src/client/rsg-components/ReactComponent/__snapshots__/ReactComponent.spec.js.snap
+++ b/src/client/rsg-components/ReactComponent/__snapshots__/ReactComponent.spec.js.snap
@@ -207,7 +207,7 @@ exports[`ReactComponent should pass rendered description, usage, examples, etc.
 
 exports[`ReactComponentRenderer should render component 1`] = `
 <div
-  id="Test-container"
+  data-testid="Test-container"
 >
   <header>
     <div>
@@ -222,7 +222,7 @@ exports[`ReactComponentRenderer should render component 1`] = `
 
 exports[`ReactComponentRenderer should render component without a pathline 1`] = `
 <div
-  id="Test-container"
+  data-testid="Test-container"
 >
   <header>
     <div>
@@ -234,7 +234,7 @@ exports[`ReactComponentRenderer should render component without a pathline 1`] =
 
 exports[`ReactComponentRenderer should render description 1`] = `
 <div
-  id="Test-container"
+  data-testid="Test-container"
 >
   <header>
     <div>
@@ -254,7 +254,7 @@ exports[`ReactComponentRenderer should render description 1`] = `
 
 exports[`ReactComponentRenderer should render docs 1`] = `
 <div
-  id="Test-container"
+  data-testid="Test-container"
 >
   <header>
     <div>
@@ -274,7 +274,7 @@ exports[`ReactComponentRenderer should render docs 1`] = `
 
 exports[`ReactComponentRenderer should render examples 1`] = `
 <div
-  id="Test-container"
+  data-testid="Test-container"
 >
   <header>
     <div>
@@ -299,7 +299,7 @@ exports[`ReactComponentRenderer should render examples 1`] = `
 
 exports[`ReactComponentRenderer should render usage section 1`] = `
 <div
-  id="Test-container"
+  data-testid="Test-container"
 >
   <header>
     <div>
diff --git a/src/client/rsg-components/StyleGuide/StyleGuideRenderer.js b/src/client/rsg-components/StyleGuide/StyleGuideRenderer.js
index 9eb29733..19183e7c 100644
--- a/src/client/rsg-components/StyleGuide/StyleGuideRenderer.js
+++ b/src/client/rsg-components/StyleGuide/StyleGuideRenderer.js
@@ -75,7 +75,7 @@ export function StyleGuideRenderer({
 				</footer>
 			</main>
 			{hasSidebar && (
-				<div className={classes.sidebar}>
+				<div className={classes.sidebar} data-testid="sidebar">
 					<header className={classes.logo}>
 						<Logo>{title}</Logo>
 						{version && <Version>{version}</Version>}
diff --git a/src/client/rsg-components/StyleGuide/__snapshots__/StyleGuide.spec.js.snap b/src/client/rsg-components/StyleGuide/__snapshots__/StyleGuide.spec.js.snap
index c2425d71..bd6056c4 100644
--- a/src/client/rsg-components/StyleGuide/__snapshots__/StyleGuide.spec.js.snap
+++ b/src/client/rsg-components/StyleGuide/__snapshots__/StyleGuide.spec.js.snap
@@ -14,7 +14,9 @@ exports[`renderer should render logo, version, table of contents, ribbon and pas
       />
     </footer>
   </main>
-  <div>
+  <div
+    data-testid="sidebar"
+  >
     <header>
       <Styled(Logo)>
         Hello
diff --git a/src/client/rsg-components/ToolbarButton/ToolbarButtonRenderer.js b/src/client/rsg-components/ToolbarButton/ToolbarButtonRenderer.js
index c8839274..29637e79 100644
--- a/src/client/rsg-components/ToolbarButton/ToolbarButtonRenderer.js
+++ b/src/client/rsg-components/ToolbarButton/ToolbarButtonRenderer.js
@@ -46,6 +46,7 @@ export function ToolbarButtonRenderer({
 	href,
 	title,
 	small,
+	testId,
 	children,
 }) {
 	const classNames = cx(classes.button, className, {
@@ -54,7 +55,7 @@ export function ToolbarButtonRenderer({
 
 	if (href !== undefined) {
 		return (
-			<a href={href} title={title} className={classNames} aria-label={title}>
+			<a href={href} title={title} className={classNames} aria-label={title} data-testid={testId}>
 				{children}
 			</a>
 		);
@@ -74,6 +75,7 @@ ToolbarButtonRenderer.propTypes = {
 	onClick: PropTypes.func,
 	title: PropTypes.string,
 	small: PropTypes.bool,
+	testId: PropTypes.string,
 	children: PropTypes.node,
 };
 
diff --git a/src/client/rsg-components/slots/IsolateButton.js b/src/client/rsg-components/slots/IsolateButton.js
index 9f32c983..ab0a65ec 100644
--- a/src/client/rsg-components/slots/IsolateButton.js
+++ b/src/client/rsg-components/slots/IsolateButton.js
@@ -5,19 +5,27 @@ import MdFullscreenExit from 'react-icons/lib/md/fullscreen-exit';
 import ToolbarButton from 'rsg-components/ToolbarButton';
 import getUrl from '../../utils/getUrl';
 
-const IsolateButton = ({ name, example, isolated }) =>
-	isolated ? (
+const IsolateButton = ({ name, example, isolated }) => {
+	const testID = example ? `${name}-${example}-isolate-button` : `${name}-isolate-button`;
+
+	return isolated ? (
 		<ToolbarButton
 			href={getUrl({ anchor: true, slug: name.toLowerCase() })}
 			title="Show all components"
+			testId={testID}
 		>
 			<MdFullscreenExit />
 		</ToolbarButton>
 	) : (
-		<ToolbarButton href={getUrl({ name, example, isolated: true })} title="Open isolated">
+		<ToolbarButton
+			href={getUrl({ name, example, isolated: true })}
+			title="Open isolated"
+			testId={testID}
+		>
 			<MdFullscreen />
 		</ToolbarButton>
 	);
+};
 
 IsolateButton.propTypes = {
 	name: PropTypes.string.isRequired,
diff --git a/src/client/rsg-components/slots/__snapshots__/IsolateButton.spec.js.snap b/src/client/rsg-components/slots/__snapshots__/IsolateButton.spec.js.snap
index b0756ad2..717e452d 100644
--- a/src/client/rsg-components/slots/__snapshots__/IsolateButton.spec.js.snap
+++ b/src/client/rsg-components/slots/__snapshots__/IsolateButton.spec.js.snap
@@ -3,6 +3,7 @@
 exports[`should renderer a link home in isolated mode 1`] = `
 <Styled(ToolbarButton)
   href="/#pizza"
+  testId="Pizza-isolate-button"
   title="Show all components"
 >
   <MdFullscreenExit />
@@ -12,6 +13,7 @@ exports[`should renderer a link home in isolated mode 1`] = `
 exports[`should renderer a link to example isolated mode 1`] = `
 <Styled(ToolbarButton)
   href="/#!/Pizza/3"
+  testId="Pizza-3-isolate-button"
   title="Open isolated"
 >
   <MdFullscreen />
@@ -21,6 +23,7 @@ exports[`should renderer a link to example isolated mode 1`] = `
 exports[`should renderer a link to isolated mode 1`] = `
 <Styled(ToolbarButton)
   href="/#!/Pizza"
+  testId="Pizza-isolate-button"
   title="Open isolated"
 >
   <MdFullscreen />
diff --git a/src/client/utils/getUrl.js b/src/client/utils/getUrl.js
index fbb6ce7a..fa0b6d80 100644
--- a/src/client/utils/getUrl.js
+++ b/src/client/utils/getUrl.js
@@ -29,15 +29,17 @@ export default function getUrl(
 		url += '?nochrome';
 	}
 
+	const encodedName = encodeURIComponent(name);
+
 	if (anchor) {
 		url += `#${slug}`;
 	} else if (isolated || nochrome) {
-		url += `#!/${name}`;
+		url += `#!/${encodedName}`;
 	}
 
 	if (hashPath) {
 		if (!id) {
-			hashPath = [...hashPath, name];
+			hashPath = [...hashPath, encodedName];
 		}
 		url += `#/${hashPath.join('/')}`;
 	}
diff --git a/src/client/utils/handleHash.js b/src/client/utils/handleHash.js
index 57b1c62b..b7b9d1f5 100644
--- a/src/client/utils/handleHash.js
+++ b/src/client/utils/handleHash.js
@@ -51,7 +51,9 @@ export const getHash = (hash, prependHash) => {
  * @return {Array.<string>}
  */
 export const getHashAsArray = (hash, prependHash) => {
-	return getHash(hash, prependHash).split(separator);
+	return trimParams(trimHash(hash, prependHash))
+		.split(separator)
+		.map(decodeURIComponent);
 };
 
 /**


Test patch:
diff --git a/src/client/utils/__tests__/getUrl.spec.js b/src/client/utils/__tests__/getUrl.spec.js
index 1c37af52..e47f4038 100644
--- a/src/client/utils/__tests__/getUrl.spec.js
+++ b/src/client/utils/__tests__/getUrl.spec.js
@@ -69,6 +69,11 @@ describe('getUrl', () => {
 		expect(result).toBe('/styleguide/#/Documentation/FooBar');
 	});
 
+	it('should return a route path with encoded name if name has inappropriate symbols', () => {
+		const result = getUrl({ name: '@foo/components', slug, hashPath: ['Documentation'] }, loc);
+		expect(result).toBe('/styleguide/#/Documentation/%40foo%2Fcomponents');
+	});
+
 	it('should return a route path with a param id=foobar', () => {
 		const result = getUrl({ name, slug, hashPath: ['Documentation'], id: true }, loc);
 		expect(result).toBe('/styleguide/#/Documentation?id=foobar');
diff --git a/src/client/utils/__tests__/handleHash.spec.js b/src/client/utils/__tests__/handleHash.spec.js
index 2cf138a0..6be00c34 100644
--- a/src/client/utils/__tests__/handleHash.spec.js
+++ b/src/client/utils/__tests__/handleHash.spec.js
@@ -44,6 +44,11 @@ describe('handleHash', () => {
 		expect(result).toEqual(['FooBar', 'Component']);
 	});
 
+	it('getHashAsArray should return array with an encoded component name', () => {
+		const result = getHashAsArray('#/Documentation/Files/%40foo%2Fcomponents', routeHash);
+		expect(result).toEqual(['Documentation', 'Files', '@foo/components']);
+	});
+
 	it('getHashAsArray should return array without params', () => {
 		const result = getHashAsArray('#/FooBar/Component?id=Example/Perfect', routeHash);
 		expect(result).toEqual(['FooBar', 'Component']);
diff --git a/test/cypress/integration/component_spec.js b/test/cypress/integration/component_spec.js
index c7625297..73860772 100644
--- a/test/cypress/integration/component_spec.js
+++ b/test/cypress/integration/component_spec.js
@@ -1,7 +1,7 @@
 describe('Single component', () => {
 	before(() => {
 		// Open simple button component in isolation
-		cy.visit('/#!/Button/1');
+		cy.visit('/#!/Button');
 	});
 
 	describe('props and methods section', () => {
@@ -35,10 +35,12 @@ describe('Single component', () => {
 
 	describe('preview section', () => {
 		beforeEach(() => {
-			cy.get('[class^=rsg--preview]')
-				.as('preview')
-				.closest('[class^=rsg--root]')
+			cy.get('[data-testid*="-example-"]')
 				.as('container')
+				.find('[class^=rsg--preview]')
+				.as('preview');
+
+			cy.get('@container')
 				.find('button')
 				.contains('View Code')
 				.as('viewCodeBtn');
@@ -79,5 +81,34 @@ describe('Single component', () => {
 				.contains('Push Me Harder')
 				.should('exist');
 		});
+
+		it('toggles isolated example mode correctly', () => {
+			cy.get('[data-testid$="-examples"]').as('componentExamples');
+
+			// Toggle into isolated example mode
+			cy.get('@componentExamples')
+				.find('[data-testid$="-isolate-button"]')
+				.first()
+				.click();
+
+			// Assert that there is only one example showing
+			cy.get('@componentExamples')
+				.find('[data-testid*="-example-"]')
+				.should('have.length', 1);
+
+			// Toggle out of isolated example mode
+			cy.get('@componentExamples')
+				.find('[data-testid$="-isolate-button"]')
+				.click();
+
+			// Assert the other examples are showing again
+			cy.get('@componentExamples')
+				.find('[data-testid*="-example-"]')
+				.should('have.length.above', 1);
+
+			// Check that we've returned to isolated component mode instead of normal mode
+			// TODO: this is currently bugged (returns to normal mode rather than isolated component mode)
+			//cy.get('[id$=container]').should('have.length', 1);
+		});
 	});
 });
diff --git a/test/cypress/integration/core_spec.js b/test/cypress/integration/core_spec.js
index bafef2c4..8e3e035b 100644
--- a/test/cypress/integration/core_spec.js
+++ b/test/cypress/integration/core_spec.js
@@ -1,18 +1,37 @@
 describe('Styleguidist core', () => {
-	beforeEach(() => cy.visit('/'));
+	before(() => cy.visit('/'));
 
 	it('loads the page', () => {
 		cy.title().should('include', 'React Styleguidist');
 	});
 
 	it('shows multiple components in normal mode', () => {
-		cy.get('[id$=container]').should('have.length.above', 1);
+		cy.get('[data-testid$=-container]').should('have.length.above', 1);
 	});
 
-	it('shows single component in isolated mode', () => {
-		cy.get('a[aria-label="Open isolated"]')
+	it('toggles isolated component mode correctly', () => {
+		cy.get('[data-testid=sidebar]').as('sidebar');
+
+		// Toggle into isolated mode
+		cy.get('[data-testid$="-isolate-button"]')
+			.first()
+			.click();
+
+		// Assert there's only one component showing
+		cy.get('[data-testid$=-container]').should('have.length', 1);
+
+		// Assert the sidebar is no longer showing
+		cy.get('@sidebar').should('not.exist');
+
+		// Toogle out of isolated mode
+		cy.get('[data-testid$="-isolate-button"]')
 			.first()
 			.click();
-		cy.get('[id$=container]').should('have.length', 1);
+
+		// Assert that more than one component is now showing
+		cy.get('[data-testid$=-container]').should('have.length.above', 1);
+
+		// Asser that the sidebar is now showing again
+		cy.get('@sidebar').should('exist');
 	});
 });