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.

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

Add custom component display names #933

Merged

Conversation

glebez
Copy link
Contributor

@glebez glebez commented Apr 10, 2018

Hey,

here comes my attempt on allowing users to define custom component display names for SG, that was discussed in #195.

Now, the user can define a custom name via a custom @sgDisplayName tag in the doc block, i.e.:

/*
* An example-less button with custom display name.
* @sgDisplayName Push Button 🎉
*/

or by utilising newly introduced updateDocs config option (#868), i.e.:

updateDocs(docs) { 
  if (docs && docs.displayName) { 
    docs.sgDisplayName = _.lowerCase(docs.displayName); 
  } 
  return docs; 
},

Let me know, please, if this is close to what you meant it to be.
Also, some updates to the cookbook/docs will follow after initial review.

Cheers,
G.

@glebez glebez changed the title Feature/custom component display names Add custom component display names Apr 10, 2018
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.

This is very cool, thank you!

My only concern is the name:

  • displayName in JS / React is a component name, so it's not very clear what it is.
  • sg suffix isn't used anywhere else. We sometimes use rsg.
  • All custom Styleguidist JSDoc tags have no prefixes (ignore, example, public).

I'd choose something like visibleName to clarify that's it's the name visible in the UI.

if (doc.doclets && doc.doclets.sgDisplayName) {
doc.sgDisplayName = doc.doclets.sgDisplayName;
delete doc.doclets.sgDisplayName;
if (doc.tags) {
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 difference between doclets and tags? Worth adding a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I don't know the difference :D I just noticed, that the custom tag gets added both to doclets and tags while parsing, so i decided it's a good idea to remove it from both. I'll investigate a little more on the matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After investigating a little, it seems that doclets and tags are a little redundant.
Here's an example of doclet:

{
  version: '1.0.1',
  author: '[Jeremy Gayed](https://github.com/tizmagik)',
  deprecated: 'Use the [only true button](#button) instead'
}

And here's the tags object:

{
  version: [ { title: 'version', description: '1.0.1' } ],
  author:
   [ { title: 'author',
       description: '[Jeremy Gayed](https://github.com/tizmagik)' } ],
  deprecated:
   [ { title: 'deprecated',
       description: 'Use the [only true button](#button) instead' } ]
}

Seems, it's the same data, structured a little differently. The interesting part, that we are using two different parsers to get those results. So, unless there are some subtle differences that I've missed, we can do some refactoring and let one of those go. But anyway, I think this work is outside of this pull request purpose. Should I file an issue for that?

@glebez
Copy link
Contributor Author

glebez commented Apr 12, 2018

I've noticed that the names of components in sidebar are still not properly updated. Will look into it.

@glebez glebez force-pushed the feature/custom-component-display-names branch from b89cdda to aab4795 Compare April 12, 2018 12:38
@glebez
Copy link
Contributor Author

glebez commented Apr 12, 2018

All right, I've changed the name, updated componenstListRenderer, so it uses visibleName as well and threw in some docs. Also, had to rebase because of conflicts with master.

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.

The code looks good, just a few tweak for the docs 🦄

docs/Cookbook.md Outdated
@@ -25,6 +25,7 @@
* [How to add fonts from Google Fonts?](#how-to-add-fonts-from-google-fonts)
* [How to reuse project’s webpack config?](#how-to-reuse-projects-webpack-config)
* [How to use React Styleguidist with Redux, Relay or Styled Components?](#how-to-use-react-styleguidist-with-redux-relay-or-styled-components)
* [How to change the names of components displayed in Styleguidist UI?](#how-to-change-the-names-of-components-displayed-in-styleguidist-ui)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's not ordered alphabetically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, it seems that the rest of the items is not ordered alphabetically as well. Should I order them?

Copy link
Member

Choose a reason for hiding this comment

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

Would be super cool! But preferably in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I finally got to alphabetically ordering the cookbook sections and want to ask: is this really a good idea? In this case, the first entry in the cookbook will be 'Are there any other projects like this?' and it feels that there are more important/useful topics in the cookbook than this... It feels that there can be a benefit in structuring the cookbook, but in some other, more semantic way, like from beginner topics to more advanced stuff or from most frequent requests to more rare/specific.

Copy link
Member

Choose a reason for hiding this comment

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

Let's skip it then ;-) Are you going to change anything else or I can merge it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right! Nope, the rest of the changes is in place already, so this one is ready to go 🚢

docs/Cookbook.md Outdated
@@ -479,6 +480,29 @@ See in [configuring webpack](Webpack.md#reusing-your-projects-webpack-config).

See [working with third-party libraries](Thirdparties.md).

## How to change the names of components displayed in Styleguidist UI?

Often you might want to change your components' names to be displayed differently, for example, for stylistic purposes or to give them a more descriptive names in your styleguide.
Copy link
Member

Choose a reason for hiding this comment

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

Not really often, I'd remove “often” ;-)

docs/Cookbook.md Outdated

This can be done by adding [@visibleName](Documenting.md#-visiblename-custom-tag) tag to your component documentation.

In case you want to change components' names in bulk, for example, based on their current name, you can utilize [updateDocs](Configuration.md#updatedocs) configuration option.
Copy link
Member

Choose a reason for hiding this comment

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

utilize → use
configuration → config

docs/Cookbook.md Outdated

In case you want to change components' names in bulk, for example, based on their current name, you can utilize [updateDocs](Configuration.md#updatedocs) configuration option.

Here's an example of how this can be achieved:
Copy link
Member

Choose a reason for hiding this comment

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

You could end the previous paragraph with a “:” and remove this one.

docs/Cookbook.md Outdated
module.exports = {
updateDocs(docs) {
if (docs && docs.displayName) {
docs.visibleName = _.lowerCase(docs.displayName)
Copy link
Member

Choose a reason for hiding this comment

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

docs.displayName.toLowerCase()

@@ -203,6 +204,23 @@ class Button extends React.Component {
}
```

### @visibleName custom tag
Copy link
Member

Choose a reason for hiding this comment

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

I think this section should be before “Using JSDoc tags”, similar to “Ignoring props”.

@@ -203,6 +204,23 @@ class Button extends React.Component {
}
```

### @visibleName custom tag

Apart from most of the jsDocs tags that can be used with RSG, @visibleName is not present in jsDoc specs. This is a custom tag, that we have added to provide users with a way to define names for components to be used in the UI.
Copy link
Member

Choose a reason for hiding this comment

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

Something like this would be shorter with the same amount of useful information:

Use @visibleName JSDoc tag to define component names that are used in the Styleguidist UI:

(And you could remove the next paragraph too.)

class Button extends React.Component {
```

By defining the @visibleName tag above, you let the Styleguidist know that this name should be used whenever the component is displayed in the UI as 'The Best Button Ever 🐙' instead of it's original 'Button' name.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of repeating what you say in the first paragraph it would be useful to say that this won't change the actual name of the component that is used in JSX.

@sapegin sapegin added this to the 7.1.0 milestone May 2, 2018
@lukashala
Copy link

Is it possible to merge this PR please? These changes would be beneficial in our project. Thx.

Copy link

@reznord reznord left a comment

Choose a reason for hiding this comment

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

@glebez, thanks for the PR 🎉, this is very cool 😄

@sapegin these changes look good to me! We can go ahead and merge this.

@codecov-io
Copy link

codecov-io commented Jul 18, 2018

Codecov Report

Merging #933 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted Files Coverage Δ
loaders/utils/getProps.js 100% <100%> (ø) ⬆️
src/utils/processComponents.js 100% <100%> (ø) ⬆️
...omponents/ComponentsList/ComponentsListRenderer.js 100% <100%> (ø) ⬆️
...rc/rsg-components/ReactComponent/ReactComponent.js 85.18% <100%> (ø) ⬆️

@sapegin
Copy link
Member

sapegin commented Jul 18, 2018

Looks like I forget to include this PR in 7.1.0, sorry 😭 — will try to make 7.2.0 soon!

@sapegin sapegin modified the milestones: 7.1.0, 7.2.0 Jul 18, 2018
@bluetidepro bluetidepro merged commit b523c45 into styleguidist:master Aug 7, 2018
bluetidepro added a commit that referenced this pull request Aug 7, 2018
## New features

* Add custom component display names ([#933](#933))

```
module.exports = {
  updateDocs(docs) {
    if (docs && docs.displayName) {
      docs.visibleName = docs.displayName.toLowerCase()
    }
    return docs
  }
}
```

* Made the mounting point id configurable. ([#1050](#1050))
bluetidepro added a commit that referenced this pull request Aug 7, 2018
## New features

* Add custom component display names ([#933](#933))

```
module.exports = {
  updateDocs(docs) {
    if (docs && docs.displayName) {
      docs.visibleName = docs.displayName.toLowerCase()
    }
    return docs
  }
}
```

* Made the mounting point id configurable. ([#1050](#1050))

## Bug fixes

* Cypress failing test ([#1077](#1077))
@styleguidist-bot
Copy link
Collaborator

🎉 This PR is included in version 7.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

sapegin pushed a commit that referenced this pull request Aug 9, 2018
- User can define a custom name via a custom @sgDisplayName tag in the doc block
valentyn90 pushed a commit to valentyn90/React-Styleguidist that referenced this pull request Oct 28, 2021
## New features

* Add custom component display names ([#933](styleguidist/react-styleguidist#933))

```
module.exports = {
  updateDocs(docs) {
    if (docs && docs.displayName) {
      docs.visibleName = docs.displayName.toLowerCase()
    }
    return docs
  }
}
```

* Made the mounting point id configurable. ([#1050](styleguidist/react-styleguidist#1050))
valentyn90 pushed a commit to valentyn90/React-Styleguidist that referenced this pull request Oct 28, 2021
## New features

* Add custom component display names ([#933](styleguidist/react-styleguidist#933))

```
module.exports = {
  updateDocs(docs) {
    if (docs && docs.displayName) {
      docs.visibleName = docs.displayName.toLowerCase()
    }
    return docs
  }
}
```

* Made the mounting point id configurable. ([#1050](styleguidist/react-styleguidist#1050))

## Bug fixes

* Cypress failing test ([#1077](styleguidist/react-styleguidist#1077))
@sapegin sapegin mentioned this pull request Aug 30, 2018
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants