-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Feat: Better anchors navigation with unique slugs #318
Conversation
- Fixes anchors for sections with multiple words in name - Fixes the case when components or sections with same name could not be resolved by anchor - Adds anchor navigation to the tittle of section/component
Codecov Report
@@ Coverage Diff @@
## next #318 +/- ##
==========================================
+ Coverage 92.58% 94.75% +2.17%
==========================================
Files 75 75
Lines 998 992 -6
Branches 203 201 -2
==========================================
+ Hits 924 940 +16
+ Misses 74 52 -22
Continue to review full report at Codecov.
|
This LGTM, although I wonder if the const genSlug = (name) => name.replace(/\s/g, '_').toLowerCase() In most cases that should do it, maybe if there's special characters or something we can put in a regex for that, etc? Just a thought! |
It is derived! Not sure what do you mean by explicitly requiring it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much!
src/utils/utils.js
Outdated
@@ -16,6 +20,22 @@ export function setComponentsNames(components) { | |||
return components; | |||
} | |||
|
|||
export function setSlugs(sections) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to extract this to a separate method like sectionSlugs
to make its singletonnes more clear. Also completely hide slugger
and export two methods: reset
and slugifySections
(since it’s not generic).
Can we do it in the styleguide-loader
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m thinking more about this: can we avoid singleton at all if we move it somewhere to utils/getSections.js
. But I’m not sure I’d work and make code better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be great. As a follow up question: do we even need to transform components and sections in the index.js
? Can this also be moved to the loader's code?
react-styleguidist/src/index.js
Lines 28 to 36 in 808fd34
let components = processComponents(styleguide.components); | |
let sections = styleguide.sections; | |
// If root `components` isn't empty, make it a first section | |
if (components.length) { | |
sections.unshift({ components }); | |
} | |
sections = processSections(sections); |
Ideally index.js should not be concerned about it IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this can be moved to loader / simplified. The only things that should be done there is globalization and name intospection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, why can't this also be done in the loader? Can we pass the reference to window
to it to make it work from one place? I assume the loader isn't a part of API so we can just use imports-plugin
for that, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can communicate with a loader only via query parameters and loaders return JS code as a string.
}); | ||
|
||
export function HeadingRenderer({ classes, children, slug, hierarchy, ...props }) { | ||
return React.createElement(`h${hierarchy}`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to use JSX?
const Tag = `h${hierarchy}`;
return (
<Tag>...</Tag>
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional variable allocation. Not even sure it's more readable. But sure, why not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it’s wasn’t a request to change, just my opinion ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried and it actually looks better in code.
className: PropTypes.string, | ||
classes: PropTypes.object.isRequired, | ||
slug: PropTypes.string.isRequired, | ||
hierarchy: PropTypes.oneOf([1, 2, 3, 4, 5, 6]).isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe level
would be a better name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a better name indeed!
Wow, this GitHub conflict merging really works! ;-) |
This doesn't bring any changes to public configurations, right? |
Yes, no changes to config. |
… name 1. Do not try to detect component name at runtime. 2. Remove nameFallback. 3. Put examples inside props. 4. Pass actual component name to default example instead of fallback. 5. Explicitly support only one component in props-loader. BREAKING CHANGE: handlers config option is now function instead of array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this should part of this PR? Seems a bit out of scope to me or what am I missing?
@okonet You asked me to do that ;-) |
[link to medium article]() ## Highlights 1. create-react-app support out of the box. 2. New webpack configuration options + user config auto load. ## Breaking changes ### create-react-app support Now Styleguidst works with [create-react-app](https://github.com/facebookincubator/create-react-app) even without config. It will load components from `src/components/**/*.js`. And example files from `Component/Readme.md` or `Component/Component.md`. ### User webpack config auto load By default Styleguidist will try to find `webpack.config.js` in your project’s root directory and use it. If your webpack config is located somewhere else, you need to load it manually: ```javascript module.exports = { webpackConfig: require('./configs/webpack.js') }; ``` > **Note:** `entry`, `externals`, `output`, `watch`, `stats` and `devtool` options will be ignored. > **Note:** `CommonsChunkPlugins`, `HtmlWebpackPlugin`, `UglifyJsPlugin`, `HotModuleReplacementPlugin` plugins will be ignored because Styleguidist already includes them or they may break Styleguidist. > **Note:** Babelified webpack configs (like `webpack.config.babel.js`) are not supported. We recommend to convert your config to native Node — Node 6 supports [many ES6 features](http://node.green/). ### Easier webpack configuration With the new [webpackConfig](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#webpackconfig): ```javascript module.exports = { webpackConfig: { module: { loaders: [ // Babel loader, will use your project’s .babelrc { test: /\.jsx?$/, exclude: /node_modules/, loader: 'babel-loader', }, // Other loaders that is needed for your components { test: /\.css$/, loader: 'style-loader!css-loader?modules', }, ], }, }, }; ``` See the new [webpack configuration guide](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Webpack.md) for more examples. Also: * `include`/`exclude` options in you webpack loaders are no longer required. * JSON loader will be added automatically if needed. ### No global Lodash in examples Lodash will not be available in examples as `_`. You can load function you need with the new [context](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#context) option: ```javascript module.exports = { context: { forEach: 'lodash/forEach', map: 'lodash/map', }, }; ``` Or replicate previous behavior though it’s not recommended: ```javascript module.exports = { context: { _: 'lodash', }, }; ``` ### Use JSS for styling instead of CSS Modules Use config option [theme](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#theme) to change fonts, colors, etc. and option [styles](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#styles) to tweak style of particular Styleguidist’s components: ```javascript module.exports = { theme: { link: 'firebrick', linkHover: 'salmon', font: '"Comic Sans MS", "Comic Sans", cursive', }, styles: { Logo: { logo: { animation: 'blink ease-in-out 300ms infinite', }, '@Keyframes blink': { to: { opacity: 0 }, }, }, }, }; ``` We now use [JSS](http://cssinjs.org/) under the hoood. ### New default config options * `components`: `src/components/**/*.js` * `getExampleFilename`: `Component/Readme.md` or `Component/Component.md` * `title`: `<app name from package.json> Style Guide` ### New default dev-server port Default port is now 6060 because create-react-app uses 3000 too. ### Use findAllExportedComponentDefinitions as a default resolver Fixes #260. ### Drop npm2 support ## Other changes and new features * New config option [require](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#require) to add new webpack entries like polyfills and custom styles * New config option [ignore](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#ignore) to exclude components from the style guide. * New config option [showSidebar](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#showsidebar) (#310) * Ignoring props with `@ignore` JSDoc tag (#353) * `objectOf` propType support (#347) * `updateWebpackConfig` option was renamed to `dangerouslyUpdateWebpackConfig` and should be used in very rare cases when `webpackConfig` isn’t enough * Style guide config validation * Reduced build size ## Bug fixes * Path for `template` config option should be relative to style guide config (#211) * Do not show isolated links on Markdown examples (#251) * Show function PropType.func’s source in a tooltip (#343) * Escape and highlight code in Markdown in descriptions (#284) * Do not change level of Markdown headings (#329) * Search should work for subsections (#245) * Better anchors navigation with unique slugs (#318) * User’s html-loader should not affect Styleguidist (#312) * Show webpack build errors and warnings * Exit with error code when build fails * Show error when no components found on style guide start * Do not fail when one of the files doesn’t export a component --- https://github.com/n1313 https://github.com/okonet https://github.com/kof
Huge thanks to @n1313, @okonet and @kof for help with this release! ## Highlights 1. create-react-app support out of the box. 2. New webpack configuration options + user config auto load. ## Breaking changes ### create-react-app support Now Styleguidst works with [create-react-app](https://github.com/facebookincubator/create-react-app) even without config. It will load components from `src/components/**/*.js`. And example files from `Component/Readme.md` or `Component/Component.md`. ### User webpack config auto load By default Styleguidist will try to find `webpack.config.js` in your project’s root directory and use it. If your webpack config is located somewhere else, you need to load it manually: ```javascript module.exports = { webpackConfig: require('./configs/webpack.js') }; ``` > **Note:** `entry`, `externals`, `output`, `watch`, `stats` and `devtool` options will be ignored. > **Note:** `CommonsChunkPlugins`, `HtmlWebpackPlugin`, `UglifyJsPlugin`, `HotModuleReplacementPlugin` plugins will be ignored because Styleguidist already includes them or they may break Styleguidist. > **Note:** Babelified webpack configs (like `webpack.config.babel.js`) are not supported. We recommend to convert your config to native Node — Node 6 supports [many ES6 features](http://node.green/). ### Easier webpack configuration With the new [webpackConfig](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#webpackconfig): ```javascript module.exports = { webpackConfig: { module: { loaders: [ // Babel loader, will use your project’s .babelrc { test: /\.jsx?$/, exclude: /node_modules/, loader: 'babel-loader', }, // Other loaders that is needed for your components { test: /\.css$/, loader: 'style-loader!css-loader?modules', }, ], }, }, }; ``` See the new [webpack configuration guide](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Webpack.md) for more examples. Also: * `include`/`exclude` options in you webpack loaders are no longer required. * JSON loader will be added automatically if needed. ### No global Lodash in examples Lodash will not be available in examples as `_`. You can load function you need with the new [context](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#context) option: ```javascript module.exports = { context: { forEach: 'lodash/forEach', map: 'lodash/map', }, }; ``` Or replicate previous behavior though it’s not recommended: ```javascript module.exports = { context: { _: 'lodash', }, }; ``` ### Use JSS for styling instead of CSS Modules Use config option [theme](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#theme) to change fonts, colors, etc. and option [styles](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#styles) to tweak style of particular Styleguidist’s components: ```javascript module.exports = { theme: { link: 'firebrick', linkHover: 'salmon', font: '"Comic Sans MS", "Comic Sans", cursive', }, styles: { Logo: { logo: { animation: 'blink ease-in-out 300ms infinite', }, '@Keyframes blink': { to: { opacity: 0 }, }, }, }, }; ``` We now use [JSS](http://cssinjs.org/) under the hoood. ### New default config options * `components`: `src/components/**/*.js` * `getExampleFilename`: `Component/Readme.md` or `Component/Component.md` * `title`: `<app name from package.json> Style Guide` ### New default dev-server port Default port is now 6060 because create-react-app uses 3000 too. ### Use findAllExportedComponentDefinitions as a default resolver Fixes #260. ### Drop npm2 support ## Other changes and new features * New config option [require](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#require) to add new webpack entries like polyfills and custom styles * New config option [ignore](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#ignore) to exclude components from the style guide. * New config option [showSidebar](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#showsidebar) (#310) * Ignoring props with `@ignore` JSDoc tag (#353) * `objectOf` propType support (#347) * `updateWebpackConfig` option was renamed to `dangerouslyUpdateWebpackConfig` and should be used in very rare cases when `webpackConfig` isn’t enough * Style guide config validation * Reduced build size ## Bug fixes * Path for `template` config option should be relative to style guide config (#211) * Do not show isolated links on Markdown examples (#251) * Show function PropType.func’s source in a tooltip (#343) * Escape and highlight code in Markdown in descriptions (#284) * Do not change level of Markdown headings (#329) * Search should work for subsections (#245) * Better anchors navigation with unique slugs (#318) * User’s html-loader should not affect Styleguidist (#312) * Show webpack build errors and warnings * Exit with error code when build fails * Show error when no components found on style guide start * Do not fail when one of the files doesn’t export a component
Huge thanks to @n1313, @okonet and @kof for help with this release! ## Highlights 1. create-react-app support out of the box. 2. New webpack configuration options + user config auto load. ## Breaking changes ### create-react-app support Now Styleguidst works with [create-react-app](https://github.com/facebookincubator/create-react-app) even without config. It will load components from `src/components/**/*.js`. And example files from `Component/Readme.md` or `Component/Component.md`. ### User webpack config auto load By default Styleguidist will try to find `webpack.config.js` in your project’s root directory and use it. If your webpack config is located somewhere else, you need to load it manually: ```javascript module.exports = { webpackConfig: require('./configs/webpack.js') }; ``` > **Note:** `entry`, `externals`, `output`, `watch`, `stats` and `devtool` options will be ignored. > **Note:** `CommonsChunkPlugins`, `HtmlWebpackPlugin`, `UglifyJsPlugin`, `HotModuleReplacementPlugin` plugins will be ignored because Styleguidist already includes them or they may break Styleguidist. > **Note:** Babelified webpack configs (like `webpack.config.babel.js`) are not supported. We recommend to convert your config to native Node — Node 6 supports [many ES6 features](http://node.green/). ### Easier webpack configuration With the new [webpackConfig](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#webpackconfig): ```javascript module.exports = { webpackConfig: { module: { loaders: [ // Babel loader, will use your project’s .babelrc { test: /\.jsx?$/, exclude: /node_modules/, loader: 'babel-loader', }, // Other loaders that is needed for your components { test: /\.css$/, loader: 'style-loader!css-loader?modules', }, ], }, }, }; ``` See the new [webpack configuration guide](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Webpack.md) for more examples. Also: * `include`/`exclude` options in you webpack loaders are no longer required. * JSON loader will be added automatically if needed. ### No global Lodash in examples Lodash will not be available in examples as `_`. You can load function you need with the new [context](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#context) option: ```javascript module.exports = { context: { forEach: 'lodash/forEach', map: 'lodash/map', }, }; ``` Or replicate previous behavior though it’s not recommended: ```javascript module.exports = { context: { _: 'lodash', }, }; ``` ### Use JSS for styling instead of CSS Modules Use config option [theme](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#theme) to change fonts, colors, etc. and option [styles](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#styles) to tweak style of particular Styleguidist’s components: ```javascript module.exports = { theme: { link: 'firebrick', linkHover: 'salmon', font: '"Comic Sans MS", "Comic Sans", cursive', }, styles: { Logo: { logo: { animation: 'blink ease-in-out 300ms infinite', }, '@Keyframes blink': { to: { opacity: 0 }, }, }, }, }; ``` We now use [JSS](http://cssinjs.org/) under the hoood. ### New default config options * `components`: `src/components/**/*.js` * `getExampleFilename`: `Component/Readme.md` or `Component/Component.md` * `title`: `<app name from package.json> Style Guide` ### New default dev-server port Default port is now 6060 because create-react-app uses 3000 too. ### Use findAllExportedComponentDefinitions as a default resolver Fixes #260. ### Drop npm2 support ## Other changes and new features * New config option [require](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#require) to add new webpack entries like polyfills and custom styles * New config option [ignore](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#ignore) to exclude components from the style guide. * New config option [showSidebar](https://github.com/styleguidist/react-styleguidist/blob/next/docs/Configuration.md#showsidebar) (#310) * Ignoring props with `@ignore` JSDoc tag (#353) * `objectOf` propType support (#347) * `updateWebpackConfig` option was renamed to `dangerouslyUpdateWebpackConfig` and should be used in very rare cases when `webpackConfig` isn’t enough * Style guide config validation * Reduced build size ## Bug fixes * Path for `template` config option should be relative to style guide config (#211) * Do not show isolated links on Markdown examples (#251) * Show function PropType.func’s source in a tooltip (#343) * Escape and highlight code in Markdown in descriptions (#284) * Do not change level of Markdown headings (#329) * Search should work for subsections (#245) * Better anchors navigation with unique slugs (#318) * User’s html-loader should not affect Styleguidist (#312) * Show webpack build errors and warnings * Exit with error code when build fails * Show error when no components found on style guide start * Do not fail when one of the files doesn’t export a component
This PR: