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

[react-jss] Fix display names with spaces inside #1049

Merged
merged 2 commits into from
Mar 6, 2019

Conversation

HenriBeck
Copy link
Member

What would you like to add/fix?
Replaces any whitespaces in a components display name with -.

Corresponding issue: #938

@HenriBeck HenriBeck requested a review from kof March 3, 2019 10:26
@@ -65,7 +65,8 @@ export default function withStyles<Theme: {}, S: Styles<Theme>>(
InnerComponent: ComponentType<Props> = NoRenderer
): ComponentType<Props> => {
const displayName = getDisplayName(InnerComponent)
const defaultClassNamePrefix = process.env.NODE_ENV === 'production' ? '' : `${displayName}-`
const defaultClassNamePrefix =
process.env.NODE_ENV === 'production' ? '' : `${displayName.replace(/\s/, '-')}-`
Copy link
Member

Choose a reason for hiding this comment

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

  • we should match all spaces, not just one, global flag is missing
  • lets add a test

Copy link
Member

@kof kof left a comment

Choose a reason for hiding this comment

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

2 things need fixing

@HenriBeck HenriBeck requested a review from kof March 6, 2019 19:39
@HenriBeck HenriBeck merged commit 50ef6cb into master Mar 6, 2019
@HenriBeck HenriBeck deleted the bugfix/react-jss/fix-spaces-in-display-name branch March 6, 2019 20:00
@kof
Copy link
Member

kof commented Mar 7, 2019

changelog 😱

bhupinderbola pushed a commit to bhupinderbola/jss that referenced this pull request Sep 17, 2019
* Fix display names with spaces inside

* Add test and replace all spaces
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.

2 participants