-
Notifications
You must be signed in to change notification settings - Fork 28
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
Added support for 'line-no-highlight' class #49
base: develop
Are you sure you want to change the base?
Conversation
… highlighting within an embed that contains highlighted lines.
@@ -477,6 +477,7 @@ public function process_gist_html( $html, array $args ) { | |||
if ( ! empty( $args['highlight'] ) ) { | |||
// Flip to use isset() when looping through the lines. | |||
$highlight = array_flip( $args['highlight'] ); | |||
$no_highlight = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the indentation on this match the preceding line? Must be using tabs to indent, not spaces.
Also, this seems to be inside the conditional that checks that there IS highlighting, which is then confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has nothing to do with space over tabs, but has everything to do with the size of tabs I'm using – will change on next commit.
Also, the $no_highlight variable is being set in there because it only applies when an embed has highlighting. I'll move it so it it simpler to understand.
…nditional check for a lines validity for the 'line-no-highlight' class so that it is easier to understand.
What you've now got is better. This will add a class to the line, rather than the overall table or div that indicates no highlighting. Is that what you're after? Why couldn't you assume that all lines were not highighted (which they aren't by default), and then only apply custom highlight styles to highlighted lines? |
That is more or less exactly what I was after, although, I agree, feature classes would be just as good a solution, if not better for the extra bit of flexibility they would offer. Here's why I did it; On an embed where I have highlighted lines, I'd prefer to fade surrounding lines out and neutralise their colours – this just further separates highlighted lines by reducing distractions. On embeds that don't have any highlighting, the lines should remain untouched. I originally knocked up a JS solution, but server-side means are better. I couldn't achieve this using adjacent sibling selectors in CSS (~), as I could only select tags that come after the targeted rule. So, if line 2 were highlighted, line 1 wouldn't be picked up by an adjacent selector. It's really just a means to targeting non-highlighted lines in amongst a gist that has highlighting. I agree, a feature class on the gist would actually be better, but I'll have to wait a few weeks before I look into that, unless someone else makes it happen in the meantime. |
@@ -495,12 +495,14 @@ public function process_gist_html( $html, array $args ) { | |||
//$classes[] = ( $key % 2 ) ? 'line-odd' : 'line-even'; | |||
$style = ''; | |||
|
|||
if ( isset( $highlight[ $key + 1 ] ) ) { | |||
if ( isset( $highlight[$key + 1] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When square brackets contain a variable, the extra spaces should be preserved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't maintain this within my IDE. The auto-formatting options in PHPStorm are simply have a space or don't have a space within brackets – there is no condition based on whether or not the content of brackets will be a variable, so I can never really accurately follow that particular piece of Wordpress coding standards. I'm wrong either way.
I'm just using the PHPStorm config settings maintained by Rarst - https://gist.github.com/Rarst/1370155
Added the 'line-no-highlight' class to lines that are within an embed containing highlighting, making it possible to specifically style non-highlighted lines. The class doesn't apply to embeds that have no highlighting.