search

bradyvercher / gistpress

WordPress plugin to add Gist oEmbed and shortcode support with caching.
GNU General Public License v2.0
143 stars 28 forks source link

Suggest adding a class to determine whether the gist has any highlighted lines #48

Open mishterk opened 10 years ago

mishterk commented 10 years ago

G'day

I've just come up against the need to target the embedded gists based on whether or not they contain highlighted lines. My goal is to fade any non-highlighted lines out, but only when there are highlighted lines to emphasize.

I can see that the table element gets a class of highlight, even when there are no lines highlighted, but if there could be a class that indicates the presence of highlighted lines, then I could easily target specific gist embeds.

mishterk commented 10 years ago

I just submitted a pull request containing a solution to this. See https://github.com/bradyvercher/gistpress/pull/49

GaryJones commented 10 years ago

I think we could expand this and target the outermost div (currently class="gist") to add in a feature / no-feature class for highlighting, shown meta, shown line-numbers etc.

mishterk commented 10 years ago

I agree, but I won't be in a position to handle that for a few weeks, as I'll be in a place that has no internet access.

I'd also suggest adding a unique ID to each instance of the embed, perhaps through the use of a custom data attribute. Currently, if the same Gist is embedded multiple times on one page, they same ID (the Gist ID) is used as the id attribute, which invalidates the HTML. Maybe we can look into moving the Gist ID into a data-gist-id attribute to allow the addition of a custom unique identifier on the shortcode.

bradyvercher commented 10 years ago

I much prefer the feature classes and would like to limit modifications to the markup as much as possible. Additional changes introduce greater potential for things to break if/when GitHub changes the markup and will make it more difficult to update in a timely manner.

Also, the highlight class in this case doesn't have anything to do with highlighted lines, rather it refers to syntax highlighting and if I recall correctly, is common to many of the syntax highlighting scripts available (it's added by GitHub in this case, not the plugin).

With that in mind, feature classes should make the line-no-highlight classes unnecessary.

GaryJones commented 10 years ago

I'm thinking of feature classes alone the lines of gist-has-meta, gist-has-line-number, gist-has-no-lines-highlighted etc. We should be able to determine which features are used, and then adapt the classes in <div class="gist"> to include the gist-has-* classes.

mishterk commented 10 years ago

I'm all for the feature classes idea, I'm just super strapped for time right now, as I'm about to go off grid for a few weeks, so I knocked up the quick solution of adding the line-no-highlight class. The gist-has-* would be ideal.