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

docs: Fix keywords for line highlighting in code blocks #18196

Merged
merged 10 commits into from Oct 14, 2019
Merged

docs: Fix keywords for line highlighting in code blocks #18196

merged 10 commits into from Oct 14, 2019

Conversation

ghost
Copy link

@ghost ghost commented Oct 5, 2019 •
edited by ghost
Loading

Description

Highlighting comment needs to be visible in the examples for Code formatting: Line highlighting. I fixed this behavior by adding the condition for highlighting only when className is not language-none. So now if you add the language keyword none to code blocks in markdown, it will stop the highlighting behavior from /www/src/components/code-block/normalize.js.

As suggested by @DSchau #17213 (comment).

See result below.

Screen Shot 2019-10-09 at 12 41 53 AM

Related PRs

#17213
#18294
@ghost ghost self-requested a review as a code owner October 5, 2019 03:54
@ghost ghost added type: documentation An issue or pull request for improving or updating Gatsby's documentation type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change labels Oct 5, 2019
@ghost ghost self-assigned this Oct 7, 2019
sidharthachatterjee
sidharthachatterjee reviewed Oct 9, 2019
View reviewed changes
docs/contributing/gatsby-style-guide.md Outdated

````
```javascript:title=gatsby-config.js
module.exports = {
siteMetadata: {
title: `GatsbyJS`,
// highlight-next-line
// keyword
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this could get confusing.

Any way we can keep the keyword and escape it so that the plugin doesn't actually highlight the snippet?
Copy link
Author

@ghost ghost Oct 9, 2019 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

@sidharthachatterjee my first thought was to include a screenshot but that seems like not a good pattern to start. I think the point of the keyword highlighting is to always highlight snippets (pretty sure it's configured in gatsby-remark-prismjs) - so really no way to turn it off?
Copy link
Contributor

Choose a reason for hiding this comment

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

We could set the language to none to disable all syntax highlighting on that snippet, I guess. What do you think?
Copy link
Author

Choose a reason for hiding this comment

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

oh good idea! let me try that
@ghost ghost requested a review from sidharthachatterjee October 9, 2019 06:21
@sidharthachatterjee
Copy link
Contributor

Looks like #17213 is relevant to this
@ghost
Copy link
Author

ghost commented Oct 9, 2019

Looks like #17213 is relevant to this

3 PR's so far! 😅will take a look
@ghost ghost self-requested a review as a code owner October 9, 2019 07:38
@sidharthachatterjee
Copy link
Contributor

@thomaswangio Looks like you might need to update a failing test
@ghost
Copy link
Author

ghost commented Oct 9, 2019 •
edited by ghost
Loading

@thomaswangio Looks like you might need to update a failing test

Yep, trying to figure that out now, spoke too soon 😅

edit: woohoo! changed empty named code blog to named none
@ghost ghost added the type: bug An issue or pull request relating to a bug in Gatsby label Oct 9, 2019
@ghost
Copy link
Author

ghost commented Oct 9, 2019

@dkthehuman @aditsachde @sidharthachatterjee @DSchau @pieh @marcysutton

tagging all contributors involved with the 3 PRs! Let me know if this solution satisfies the issue 😇
@wardpeet
Copy link
Contributor

what about using no-highlight instead of just none? (sorry for the bikeshed)
@ghost
Copy link
Author

ghost commented Oct 14, 2019

what about using no-highlight instead of just none? (sorry for the bikeshed)

thanks @wardpeet! that's probably more explicit, updated!
sidharthachatterjee
sidharthachatterjee approved these changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee 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 great! Thank you so much, Thomas 🥇
@sidharthachatterjee sidharthachatterjee changed the title [docs] Fixing keywords for line highlighting in code blocks Oct 14, 2019
@sidharthachatterjee sidharthachatterjee merged commit 277e86f into gatsbyjs:master Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby type: documentation An issue or pull request for improving or updating Gatsby's documentation type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
3 participants
@sidharthachatterjee @wardpeet @marcysutton