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

Add language slug and name to <pre> wrapper element #208

Merged
merged 7 commits into from
Oct 27, 2020

Conversation

westonruter
Copy link
Owner

Fixes #37.

This PR adds two new data attributes to the pre.wp-block-code element:

  • data-language-slug: The identifier for the language.
  • data-language-name: The translated name (label) for the language.

Given this post_content:

<!-- wp:code {"highlightedLines":"2"} -->
<pre class="wp-block-code"><code>if ( a &lt; b ) {
   alert( 'A is less than B!' );
}</code></pre>
<!-- /wp:code -->

Before:

<pre class="wp-block-code"><div><code class="hljs language-javascript shcb-code-table"><span class='shcb-loc'><span><span class="hljs-keyword">if</span> ( a &lt; b ) {
</span></span><mark class='shcb-loc'><span>   alert( <span class="hljs-string">'A is less than B!'</span> );
</span></mark><span class='shcb-loc'><span>}
</span></span></code></div></pre>

After:

<pre class="wp-block-code" data-language-slug="javascript" data-language-name="JavaScript"><div><code class="hljs language-javascript shcb-code-table"><span class='shcb-loc'><span><span class="hljs-keyword">if</span> ( a &lt; b ) {
</span></span><mark class='shcb-loc'><span>   alert( <span class="hljs-string">'A is less than B!'</span> );
</span></mark><span class='shcb-loc'><span>}
</span></span></code></div></pre>

Copy link
Collaborator

@allejo allejo left a comment

Choose a reason for hiding this comment

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

I like it! 👍

@westonruter
Copy link
Owner Author

The only thing that gives me pause is that perhaps it would be better if the data-language-name were actually added as an element, perhaps as a span in the first child. This would allow for the language name to actually be accessible to screen readers and maybe for crawlers.

@allejo
Copy link
Collaborator

allejo commented Oct 27, 2020

If it's added as a span, then should it be opt-in? My only reservation about it being in the DOM is that we now have to style it. The current styling we introduce is minimal (hljs + layout for line numbers) but if we style how this span looks then users would need to reset its styles for their own themes or hide it entirely if they don't want it.

@westonruter
Copy link
Owner Author

What if we display:none it by default?

@allejo
Copy link
Collaborator

allejo commented Oct 27, 2020

+1 for display: none! would it be two spans? one for the language name and one for the slug?

@westonruter
Copy link
Owner Author

I suppose it depends on the use case. I was thinking that the language name would be displayed instead of the language slug. But then what purpose does having the language slug as an attribute? If both were added as hidden span elements then the author could decide.

@allejo
Copy link
Collaborator

allejo commented Oct 27, 2020

I think two span elements and letting users decide which they want to appear would be best

@westonruter
Copy link
Owner Author

westonruter commented Oct 27, 2020

Oh hold on. Adding inline elements will cause problems when the block is rendered and the HTML is served in RSS feeds or email clients, where the styles may get stripped out. We'd have to make sure that the span looks OK even when no styling is applied. So take a look at e08ff6e. With that, the HTML is:

<pre class="wp-block-code" aria-describedby="shcb-language-3" data-shcb-language-name="JavaScript" data-shcb-language-slug="javascript"><div><code class="hljs language-javascript shcb-code-table"><span class='shcb-loc'><span><span class="hljs-keyword">if</span> ( a &lt; b ) {
</span></span><mark class='shcb-loc'><span>   alert( <span class="hljs-string">'A is less than B!'</span> );
</span></mark><span class='shcb-loc'><span>}
</span></span></code></div><small class="shcb-language" id="shcb-language-3"><span class="shcb-language__label">Code language:</span> <span class="shcb-language__name">JavaScript</span> <span class="shcb-language__paren">(</span><span class="shcb-language__slug">javascript</span><span class="shcb-language__paren">)</span></small></pre>

When styles are present, the language is hidden by default:

image

When styles are removed:

image

And when custom styles are added to display the language info in the desired location:

image

For example:

.wp-block-code {
	position: relative;
}

.wp-block-code .shcb-language {
	clip: initial;
	width: auto;
	height: auto;
	margin: auto;
	clip-path: none;

	top: 6px;
	right: 6px;
	line-height: 1em;
	white-space: normal;
	font-family: sans-serif;
	font-size: 12px;
	font-weight: bold;
}

.wp-block-code .shcb-language__label,
.wp-block-code .shcb-language__paren,
.wp-block-code .shcb-language__slug {
	display: none;
}

@westonruter
Copy link
Owner Author

westonruter commented Oct 27, 2020

For maximum styling flexibility, the language should probably be added to data-* attributes as well. Done in 56f379d.

@westonruter
Copy link
Owner Author

Updated #208 (comment) to show how aria-describedby is now being used.

Comment on lines +10 to +23
.shcb-language {
border: 0;
clip: rect(1px, 1px, 1px, 1px);
-webkit-clip-path: inset(50%);
clip-path: inset(50%);
height: 1px;
margin: -1px;
overflow: hidden;
padding: 0;
position: absolute;
width: 1px;
word-wrap: normal;
word-break: normal;
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is adapted from the ubiquitous screen-reader-text styles used in WordPress.

@allejo
Copy link
Collaborator

allejo commented Oct 27, 2020

Ah this is a lot better! I like how even the parentheses are surrounded by classes so they can be visually hidden.

Which properties need to be reset in shcb-language will need to be documented in the wiki.

@westonruter westonruter merged commit f77e041 into develop Oct 27, 2020
@westonruter westonruter deleted the add/language-attributes branch October 27, 2020 23:57
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.

Add selected language as a data- attribute in rendered code block
2 participants