Section isolated mode #394

Knorcedger · 2017-04-13T14:32:20Z

This is the implementation of the section isolated mode.

I was not able to write tests for it because I couldn't run the tests. I think it's something with react-addons-test-utils moving to react-dom/test-utils. Is this fixed in the master branch, or can someone take a look so that I can run the tests and add tests for this too?

sapegin · 2017-04-13T14:42:14Z

Thanks!

I’m trying to merge #389 but it still doesn’t work as it should and I’m not sure why ;-|

sapegin · 2017-04-13T14:46:08Z

src/rsg-components/Section/SectionRenderer.js

+				{name && (
+					<Heading level={1} slug={slug} className={classes.heading}>{name}</Heading>
+				)}
+				<div className={classes.isolatedLink}>


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

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);

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>

@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?

Yes please!

sapegin · 2017-04-13T14:46:57Z

src/utils/utils.js

@@ -148,22 +148,26 @@ export function filterComponentsInSectionsByExactName(sections, name) {
 	return components;
 }

+export function filterSections(sections, name) {


Please add JDoc here.

Knorcedger · 2017-04-13T15:02:23Z

Cool, I will wait for the other PR to be merged first and then add the tests. In the mean time will fix the Changes Requested :)

sapegin · 2017-04-13T15:03:38Z

Awesome! 🍕

sapegin · 2017-04-14T10:35:25Z

Tests in master should work now.

Knorcedger · 2017-04-25T13:51:24Z

@sapegin updated the PR. Let me know if you want me to clarify anything. Tests are still missing

P.S. Also, after merging master in, everything has a purple border, not sure why.

sapegin · 2017-04-25T17:41:04Z

That’s what I’d do: https://gist.github.com/9d02a2603619daa8de7f1287490c894f

Knorcedger · 2017-05-03T13:01:43Z

I tried your proposed solution but that doesn't solve the problem that to open an isolated example you have to hover over the "opacity: 0" link.

sapegin · 2017-05-03T13:04:13Z

You never hover on links, only on containers.

sapegin · 2017-05-03T13:10:30Z

But probably you can hardcode these links back because of #426 ;-)

Knorcedger · 2017-05-03T15:56:26Z

I think we have a communication problem, and that's why I updated the PR with the changes you suggested. If you run the project with npm run start:sections and go to the first Button example that says 'Push Me' the 'Open Isolated' but has opacity 0, and the only way to reveal it, it to hover over the actual invisible button, and not the whole container. That was the initial problem and I don't know how to fix.

sapegin · 2017-05-03T16:40:34Z

Works fine for me:

But please take a look at #426, we’re probably trying to do something that would be removed soon. So if it‘s easier to revert, please revert it back, and sorry for inconsistent suggestions ;-)

Knorcedger · 2017-05-04T10:16:46Z

Ok, cool. Shall I revert back to this commit and then add the tests?

sapegin · 2017-05-04T11:26:08Z

Yep, thanks! 🍕

codecov-io · 2017-05-09T16:24:46Z

Codecov Report

Merging #394 into master will decrease coverage by 0.22%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##           master     #394      +/-   ##
==========================================
- Coverage   94.28%   94.06%   -0.23%     
==========================================
  Files          82       82              
  Lines        1138     1145       +7     
  Branches      255      260       +5     
==========================================
+ Hits         1073     1077       +4     
- Misses         61       64       +3     
  Partials        4        4

Impacted Files	Coverage Δ
src/rsg-components/StyleGuide/StyleGuide.js	`100% <ø> (ø)`	⬆️
src/rsg-components/Section/Section.js	`100% <100%> (ø)`	⬆️
src/utils/utils.js	`93.93% <50%> (-2.94%)`	⬇️
src/rsg-components/Section/SectionRenderer.js	`90% <85.71%> (-10%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update feaa6b8...f2ddd58. Read the comment docs.

Knorcedger · 2017-05-09T16:34:18Z

@sapegin I reverted to the commit you suggested and tried to adjust the tests, but I'm not familiar with Jest and it's snapshots so I'm not sure if it's 100% ok. At least npm test was reporting no errors locally :)

sapegin · 2017-05-09T16:43:21Z

src/rsg-components/Section/__snapshots__/Section.spec.js.snap

@@ -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>


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

sapegin · 2017-05-09T16:44:25Z

src/utils/utils.js

+ * @param  {string} name     The name to match
+ * @return {object}          The section found
+ */
+export function filterSections(sections, name) {


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

I added tests for this function.

sapegin · 2017-05-09T16:45:02Z

src/rsg-components/Section/SectionRenderer.js

+				<div className={classes.isolatedLink}>
+					{name && (
+						isolatedSection ? (
+							<Link href="/">⇽ Back</Link>


And a test for back link.

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

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

sapegin · 2017-05-09T16:45:41Z

Looks fine! There are two pieces not covered by any tests though.

@Knorcedger

## New feature **Section isolated mode:** now you can open sections in isolated mode too. (#394 by @Knorcedger) ## Bug fixes * Fix React CodeMirror warning when opening code editor * Fix Create React App 1.0 support (#458) * Fix back button path for support pathname-relative paths (#447 by @stepancar) * Promise polyfill for IE11 because require.ensure requires it in webpack 2

Section isolated mode

d24d937

Knorcedger mentioned this pull request Apr 13, 2017

Render a Section in Isolated mode #333

Closed

sapegin requested changes Apr 13, 2017

View reviewed changes

Added JSDoc in utils -> filterSections

de6dd2d

Knorcedger force-pushed the section-isolated-mode branch from 46ab135 to de6dd2d Compare May 9, 2017 15:22

Merge master and fix tests

c0a9856

sapegin reviewed May 9, 2017

View reviewed changes

Added tests for utils -> filterSections method

f2ddd58

sapegin merged commit 4de939d into styleguidist:master May 11, 2017

sapegin pushed a commit that referenced this pull request May 12, 2017

Feat: Section isolated mode (#394)

db815e5

kopax mentioned this pull request Nov 14, 2023

[Snyk] Fix for 1 vulnerabilities rollup-umd/react-styleguidist#152

Open

Bhanditz mentioned this pull request Nov 15, 2023

[Snyk] Security upgrade copy-webpack-plugin from 4.3.0 to 5.0.5 Bhanditz/react-styleguidist#186

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Section isolated mode #394

Section isolated mode #394

Knorcedger commented Apr 13, 2017

sapegin commented Apr 13, 2017

sapegin Apr 13, 2017

Knorcedger Apr 19, 2017

sapegin Apr 20, 2017

Knorcedger Apr 24, 2017

sapegin Apr 24, 2017

sapegin Apr 13, 2017

Knorcedger commented Apr 13, 2017

sapegin commented Apr 13, 2017

sapegin commented Apr 14, 2017

Knorcedger commented Apr 25, 2017 •

edited

Loading

sapegin commented Apr 25, 2017

Knorcedger commented May 3, 2017

sapegin commented May 3, 2017

sapegin commented May 3, 2017

Knorcedger commented May 3, 2017

sapegin commented May 3, 2017

Knorcedger commented May 4, 2017

sapegin commented May 4, 2017

codecov-io commented May 9, 2017 •

edited

Loading

Knorcedger commented May 9, 2017

sapegin May 9, 2017

sapegin May 9, 2017

Knorcedger May 11, 2017

sapegin May 9, 2017

Knorcedger May 11, 2017

sapegin May 11, 2017

sapegin commented May 9, 2017 •

edited

Loading

Section isolated mode #394

Section isolated mode #394

Conversation

Knorcedger commented Apr 13, 2017

sapegin commented Apr 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Knorcedger commented Apr 13, 2017

sapegin commented Apr 13, 2017

sapegin commented Apr 14, 2017

Knorcedger commented Apr 25, 2017 • edited Loading

sapegin commented Apr 25, 2017

Knorcedger commented May 3, 2017

sapegin commented May 3, 2017

sapegin commented May 3, 2017

Knorcedger commented May 3, 2017

sapegin commented May 3, 2017

Knorcedger commented May 4, 2017

sapegin commented May 4, 2017

codecov-io commented May 9, 2017 • edited Loading

Codecov Report

Knorcedger commented May 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sapegin commented May 9, 2017 • edited Loading

Knorcedger commented Apr 25, 2017 •

edited

Loading

codecov-io commented May 9, 2017 •

edited

Loading

sapegin commented May 9, 2017 •

edited

Loading