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

JSX/TSX: Tags of React components are styled as classes #1519

Merged
merged 2 commits into from
Aug 20, 2018

Conversation

RunDevelopment
Copy link
Member

This PR implements #1518 but it uses class-name in favor of the proposed tag-class. Like this, we don't have to change the themes.

The PR adds a class-name alias to all the tags of React components. Tags which start with an uppercase letter are considered React components.
HTML tags remain unchanged.

The new styling is similar to VS Code's.

Example: (taken from the React tutorial)
image

VS Code:
image

@mAAdhaTTah
Copy link
Member

Hey @RunDevelopment – Just wanted to say I appreciate the reviews & PRs you've made to Prism. I haven't had a lot of time to dig into all of them, and a lot of the regex changes I'm unfortunately not really qualified to review, but I just wanted to let you know we see the work you've done and thank you for it.

@Templarian
Copy link

@RunDevelopment Thanks for writing this so quickly, exactly what I needed. The code/results looks good to me. 😃

@atomiks
Copy link

atomiks commented Aug 14, 2018

Could this be changed to tag-component or tag-class instead of class-name. The reason is to desire as much granularity with the highlighting as possible, if we don't want the JavaScript class color to match the component tag color.

@Templarian
Copy link

@atomiks The theme will just use a more specific CSS selector.

.token.tag.class-name {
    color: red;
}

@atomiks
Copy link

atomiks commented Aug 14, 2018

Ah ok, looks good then!

@mAAdhaTTah
Copy link
Member

Can we get tests for this?

@RunDevelopment
Copy link
Member Author

Unfortunately, it is not possible to test this feature with our usual tests.
Because this PR only adds an alias to a token, the alias will not show up in the JSON (see jsx/tag_feature.test which I did not have to change, but is styled like a class in the test page.).

@mAAdhaTTah
Copy link
Member

But you could confirm that <Button /> styles Button as a class-name?

@RunDevelopment
Copy link
Member Author

RunDevelopment commented Aug 19, 2018

But you could confirm that styles Button as a class-name?

Depends:
With the current implementation of this PR and the test suite, definitely no.

But, we could change the implementation of this PR so that it will replace the string Button with a new token with the type class-name. instead of adding an alias to the tag token. That will be testable.

@mAAdhaTTah
Copy link
Member

I dig this, much closer to what I was thinking.

@mAAdhaTTah mAAdhaTTah merged commit 3e1a9a3 into PrismJS:master Aug 20, 2018
@RunDevelopment RunDevelopment deleted the jsx-class-name branch August 20, 2018 14:31
@RunDevelopment
Copy link
Member Author

One last note:

Because the implementation is now different, the CSS selector to highlight the class-name token of tags has to look like this:

.token.tag > .token.class-name {
    color: red;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants