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

feat: 'fork me' ribbon #861

Merged
merged 7 commits into from
Mar 21, 2018
Merged

feat: 'fork me' ribbon #861

merged 7 commits into from
Mar 21, 2018

Conversation

glebez
Copy link
Contributor

@glebez glebez commented Mar 6, 2018

Hi,

this PR is related to #195 issue and builds up on PR #647, that didn't show any activity for a while.

I've rebased wkwiatek's branch and added some tweaks based on feedback in the above PR.

Hope I got the requirements right and let me know, please, if any other changes are needed.

sapegin
sapegin requested changes Mar 7, 2018
View reviewed changes
Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Thanks! Couple of small things ;-)

src/rsg-components/Ribbon/index.js Outdated
@@ -0,0 +1 @@
export { default } from './Ribbon.js';
Copy link
Member

Choose a reason for hiding this comment

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

Here should be a full path with rsg-components too.

src/rsg-components/StyleGuide/StyleGuideRenderer.js Outdated
@@ -4,6 +4,7 @@ import Logo from 'rsg-components/Logo';
import Markdown from 'rsg-components/Markdown';
import Styled from 'rsg-components/Styled';
import cx from 'classnames';
import Ribbon from '../Ribbon';
Copy link
Member

Choose a reason for hiding this comment

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

And here.

src/rsg-components/Ribbon/RibbonRenderer.js Outdated
classes: PropTypes.object.isRequired,
url: PropTypes.string.isRequired,
text: PropTypes.string,
color: PropTypes.string,
Copy link
Member

Choose a reason for hiding this comment

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

You don't use color and background props.

src/rsg-components/Ribbon/RibbonRenderer.js Outdated
return (
<div className={classes.root}>
<a href={url} className={classes.link}>
{text || 'Fork me on GitHub'}
Copy link
Member

Choose a reason for hiding this comment

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

This could be a defaultProps.

src/rsg-components/Ribbon/RibbonRenderer.js Outdated
top: -22,
display: 'block',
width: 190,
padding: '5px 15px',
Copy link
Member

Choose a reason for hiding this comment

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

Try to use space variables from theme here. (I'm not sure it's a good idea, but worth trying ;-)

@glebez
Copy link
Contributor Author

glebez commented Mar 7, 2018

Aaand the fixes are there.

sapegin
sapegin requested changes Mar 12, 2018
View reviewed changes
Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Couple of tiny things, and we're done here 🦄

src/rsg-components/Ribbon/RibbonRenderer.js Outdated
top: -22,
display: 'block',
width: 190,
padding: `${space[0]}px ${space[2]}px`,
Copy link
Member

Choose a reason for hiding this comment

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

[[space[0], space[2]]] is a preferred way (note double brackets).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, cool, thx, should have read more on jss before jumping on styles ;) will fix

src/rsg-components/Ribbon/RibbonRenderer.js Outdated
position: 'fixed',
top: 0,
right: 0,
width: '149px',
Copy link
Member

Choose a reason for hiding this comment

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

Should work without px.

glebez
glebez commented Mar 12, 2018
View reviewed changes
src/rsg-components/Ribbon/RibbonRenderer.js
fontSize: fontSize.base,
background: color.ribbonBackground,
textDecoration: 'none',
textShadow: [[0, '-1px', 0, 'rgba(0,0,0,.15)']],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to use px here, without it the code was compiling to text-shadow: 0 -1 0 rgba(0,0,0,.15) and browser marked it as unknown value.

@glebez
Copy link
Contributor Author

glebez commented Mar 12, 2018

Ok, style fixes are there.

@sapegin sapegin added this to the 6.3.0 milestone Mar 13, 2018
@sapegin
Copy link
Member

sapegin commented Mar 13, 2018

Awesome, thanks!

@sapegin sapegin merged commit 5eaae36 into styleguidist:master Mar 21, 2018
sapegin pushed a commit that referenced this pull request Mar 22, 2018
Changelog: 🚀
a5cd8f4 
👋 **[Support Styleguidist](https://opencollective.com/styleguidist) on Open Collective** 👋

## New features

### Page per section 🚧

This is a huge improvement for large style guides: now you can show only one component at a page instead of all components.

![](https://d3vv6lp55qjaqc.cloudfront.net/items/0W2q2K2s3n2k311O1J0y/Screen%20Recording%202018-03-22%20at%2010.06%20AM.gif)

To enable:

```js
module.exports = {
  pagePerSection: true
};
```

This is an experimental feature and we need your feedback to make it better and really useful. For example, right now there’s no isolated mode. So feel free to share your ideas [in issues](https://github.com/styleguidist/react-styleguidist/issues).

(#835 by @bitionaire and @stepancar, closes #494 and #768)

### “Fork me” ribbon

One more step to make Styleguidist usable for documenting open source projects:

![](https://d3vv6lp55qjaqc.cloudfront.net/items/1t331n2a3v0F2i290K0Z/Image%202018-03-22%20at%209.13.11%20AM.png)

New config option to enable the ribbon, define the URL and change the label:

```js
module.exports = {
  ribbon: {
    url: 'http://example.com/',
    text: 'Fork me on GitHub'
  }
};
```

And two new [theme variables](https://github.com/styleguidist/react-styleguidist/blob/master/src/styles/theme.js) to change colors: `color.ribbonBackground` and `color.ribbonText`.

(#861 by @glebez and @wkwiatek, part of #195, closes #647)

### Options to change CLI output on server start and build

Two new options, `printServerInstructions` and `printBuildInstructions`:

```js
module.exports = {
  printBuildInstructions(config) {
    console.log(
      `Style guide published to ${
        config.styleguideDir
      }. Something else interesting.`
    );
  }
};
```

(#878 by @roblevintennis, closes #876)

### Option to modify props

A new option, `updateDocs` to modify props before rendering. For example, you can load a component version number from a JSON file:

```js
module.exports = {
  updateDocs(docs) {
    if (docs.doclets.version) {
      const versionFilePath = path.resolve(
        path.dirname(file),
        docs.doclets.version
      );
      const version = require(versionFilePath).version;

      docs.doclets.version = version;
      docs.tags.version[0].description = version;
    }

    return docs;
  }
};
```

(#868 by @ryanoglesby08)

### Limited support for named exports

Styleguidist used to require default exports for components. Now you can use named exports too, though Styleguidist still supports only one component per file. I hope this change will make some users happier, see more details [in the docs](https://react-styleguidist.js.org/docs/components.html#loading-and-exposing-components).

(#825 by @marcdavi-es, closes #820 and #633)

### Allow arrays of component paths in sections

More flexibility in structuring your style guide:

```js
module.exports = {
  sections: [
    {
      name: 'Forms',
      components: [
        'lib/components/ui/Button.js',
        'lib/components/ui/Input.js'
      ]
    }
  ]
};
```

(#794 by @glebez, closes #774)

### Sort properties by `required` and `name` attributes

This change should make props tables more predictable for the users. Instead of relying on developers to sort props in a meaningful way, Styleguidist will show required props first, and sort props in each group alphabetically.

To disable sorting, use the identity function:

```javascript
module.exports = {
  sortProps: props => props
};
```

(#784 by @dotcs)

## Bug fixes

* Allow `Immutable` state in examples (#870 by @nicoffee, closes #864)
* Change template container ID to prevent clashes (#859 by @glebez, closes #753)
* Better props definitions with Flow types (#781 by @nanot1m)
@sapegin
Copy link
Member

sapegin commented Mar 22, 2018

🎉 This PR is included in version 6.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

This was referenced Jul 17, 2020
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sapegin sapegin sapegin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.5.0
Development

Successfully merging this pull request may close these issues.
3 participants
@glebez @sapegin @wkwiatek