Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added support for external links in sections #921

Merged
merged 15 commits into from
Aug 27, 2018
Merged

Added support for external links in sections #921

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d7838f9
Added support for external links in sections
BTMPL Apr 4, 2018
1c9d38a
Removed the skip functionality, changed external prop to href prop
BTMPL Apr 9, 2018
8231ef2
Removed leftover skip and external prop
BTMPL Apr 9, 2018
1855b1f
Renamed test for target attribute
BTMPL Apr 9, 2018
982fe46
Reverted change to docs
BTMPL Apr 9, 2018
67de765
Resolve merge conflicts, update implementation
BTMPL Aug 12, 2018
09029e7
Update example sections config and fialing tests
BTMPL Aug 12, 2018
65a8e53
Use a unique string as Section key
BTMPL Aug 12, 2018
2fff2bd
Reverted change to Sections component; apparently there are cases whe…
BTMPL Aug 12, 2018
4bea88e
Reverted package-lock.json
BTMPL Aug 12, 2018
18fdf4e
Revert Cookbook.md
BTMPL Aug 12, 2018
61cfc4e
Properly unstage Cookbook
BTMPL Aug 12, 2018
34ee0b4
The cookbook is changed by pre-commit hook
BTMPL Aug 12, 2018
a946379
Added test for the href prop
BTMPL Aug 13, 2018
4d7031a
Changed jsx to markdown syntax
BTMPL Aug 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/Components.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ Each section consists of (all fields are optional):
- `exampleMode` — Initial state of the code example tab, uses [exampleMode](Configuration.md#examplemode).
- `usageMode` — Initial state of the props and methods tab, uses [usageMode](Configuration.md#usagemode).
- `ignore` — string/array of globs that should not be included in the section.
- `href` - an URL to navigate to instead of navigating to the section content
- `external` - if set, the link will open in a new window

Configuring a style guide with textual documentation section and a list of components would look like:

Expand All @@ -134,6 +136,11 @@ module.exports = {
{
name: 'Configuration',
content: 'docs/configuration.md'
},
{
name: 'Live Demo',
external: true,
href: 'http://example.com'
}
]
},
Expand Down
2 changes: 1 addition & 1 deletion docs/Cookbook.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export default icons

````jsx
// IconGallery.md
```jsx noeditor
;```jsx noeditor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's going on here; this looks to be changed by a pre-commit hook, and probably should?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the file was edited in the browser so the precommit hook wasn't run ;-/ Could you please change jsx to markdown for the whole block? It'll stop Prettier from adding ;.

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 assume jsx static won't work here? Updating.

Copy link
Member

Choose a reason for hiding this comment

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

It's a bit confusing: here's an example of what you should write in Markdown, so the outer tag should be markdown instead of jsx. jsx noeditor part is fine — it's part of an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was thinking ````jsx static would work in order to keep highlights. Should be like you asked now; I think the newline was added by precommit hook again.

Copy link
Member

Choose a reason for hiding this comment

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

New line is fine!

const icons = require('./load-icons').default

const iconElements = Object.keys(icons).map(iconName => {
Expand Down
5 changes: 5 additions & 0 deletions examples/sections/styleguide.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ module.exports = {
},
],
},
{
name: 'Online documentation',
href: 'https://github.com/styleguidist/react-styleguidist',
external: true,
},
],
sectionDepth: 2,
},
Expand Down
12 changes: 12 additions & 0 deletions loaders/utils/__tests__/__snapshots__/getSections.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ Array [
},
"description": undefined,
"exampleMode": "collapse",
"external": undefined,
"filepath": "components/Button/Readme.md",
"href": undefined,
"name": "Readme",
"sectionDepth": 0,
"sections": Array [],
Expand Down Expand Up @@ -89,7 +91,9 @@ Array [
"content": undefined,
"description": undefined,
"exampleMode": "collapse",
"external": undefined,
"filepath": undefined,
"href": undefined,
"name": "Components",
"sectionDepth": 0,
"sections": Array [],
Expand Down Expand Up @@ -156,7 +160,9 @@ Array [
"content": undefined,
"description": undefined,
"exampleMode": "collapse",
"external": undefined,
"filepath": undefined,
"href": undefined,
"name": "Ignore",
"sectionDepth": 0,
"sections": Array [],
Expand Down Expand Up @@ -240,7 +246,9 @@ Object {
"content": undefined,
"description": undefined,
"exampleMode": "collapse",
"external": undefined,
"filepath": undefined,
"href": undefined,
"name": "Components",
"sectionDepth": 0,
"sections": Array [],
Expand All @@ -257,7 +265,9 @@ Object {
},
"description": undefined,
"exampleMode": "collapse",
"external": undefined,
"filepath": "components/Button/Readme.md",
"href": undefined,
"name": "Readme",
"sectionDepth": 0,
"sections": Array [],
Expand Down Expand Up @@ -327,7 +337,9 @@ Object {
"content": undefined,
"description": undefined,
"exampleMode": "collapse",
"external": undefined,
"filepath": undefined,
"href": undefined,
"name": "Ignore",
"sectionDepth": 0,
"sections": Array [],
Expand Down
2 changes: 2 additions & 0 deletions loaders/utils/getSections.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ function processSection(section, config, parentDepth) {
slug: slugger.slug(section.name),
sections: getSections(section.sections || [], config, sectionDepth),
filepath: contentRelativePath,
href: section.href,
components: getSectionComponents(section, config),
content,
external: section.external,
};
}

Expand Down
16 changes: 9 additions & 7 deletions src/rsg-components/ComponentsList/ComponentsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import getUrl from '../../utils/getUrl';
function ComponentsList({ classes, items, useRouterLinks = false, useHashId, hashPath }) {
const mappedItems = items.map(item => ({
...item,
href: getUrl({
name: item.name,
slug: item.slug,
anchor: !useRouterLinks,
hashPath: useRouterLinks ? hashPath : false,
id: useRouterLinks ? useHashId : false,
}),
href: item.href
? item.href
: getUrl({
name: item.name,
slug: item.slug,
anchor: !useRouterLinks,
hashPath: useRouterLinks ? hashPath : false,
id: useRouterLinks ? useHashId : false,
}),
}));
return <ComponentsListRenderer classes={classes} items={mappedItems} />;
}
Expand Down
8 changes: 6 additions & 2 deletions src/rsg-components/ComponentsList/ComponentsListRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,16 @@ export function ComponentsListRenderer({ classes, items }) {

return (
<ul className={classes.list}>
{items.map(({ heading, visibleName, href, content }) => (
{items.map(({ heading, visibleName, href, content, external }) => (
<li
className={cx(classes.item, (!content || !content.props.items.length) && classes.isChild)}
key={href}
>
<Link className={cx(heading && classes.heading)} href={href}>
<Link
className={cx(heading && classes.heading)}
href={href}
target={external ? '_blank' : undefined}
>
{visibleName}
</Link>
{content}
Expand Down
10 changes: 10 additions & 0 deletions src/rsg-components/Link/Link.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,13 @@ it('should compose passed class names', () => {

expect(actual.find('a').prop('className')).toBe('baseLinkClass customClass');
});

it('should properly pass the target attribute', () => {
const actual = shallow(
<LinkRenderer href={href} target="_blank" classes={{}}>
{children}
</LinkRenderer>
);

expect(actual.find('a').prop('target')).toBe('_blank');
});
4 changes: 3 additions & 1 deletion src/rsg-components/Sections/Sections.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import SectionsRenderer from 'rsg-components/Sections/SectionsRenderer';
export default function Sections({ sections, depth }) {
return (
<SectionsRenderer>
{sections.map((section, idx) => <Section key={idx} section={section} depth={depth} />)}
{sections
.filter(section => !section.href)
Copy link
Member

Choose a reason for hiding this comment

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

What's the use case for sections without href?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently any section added to the configuration will result in a section space being generated in the main content - this would also include the href sections which should open in new window.

This will filter out those sections from being rendered in the main content area.

.map((section, idx) => <Section key={idx} section={section} depth={depth} />)}
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we can improve key={idx} now? Is there anything unique for each section that we can use as a key instead of an index?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dont thinks so, unless we assume that all sections have unique hrefs, which they might now have, so I guess we are stuck with the index for now.

Copy link
Member

Choose a reason for hiding this comment

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

Then title + href should be unique enough, I'd really like to avoid indexes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out that some sections do not have slug, but are rendered with an array of components - this is for example visible in sections example, WrappedButton link.

</SectionsRenderer>
);
}
Expand Down