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

[Feature] Enhanced code blocks #131

Merged
merged 28 commits into from
Oct 9, 2021
Merged

[Feature] Enhanced code blocks #131

merged 28 commits into from
Oct 9, 2021

Conversation

seshrs
Copy link
Member

@seshrs seshrs commented Sep 29, 2021

(Closes #129, closes #130.)

TLDR

Primer Spec now enhances all code blocks (including standard HTML <pre> blocks). Check out the screenshots/videos below, or try them out at the preview URL: https://preview.sesh.rs/previews/eecs485staff/primer-spec/131/demo/enhanced-code-blocks.html

Context

Code blocks are a core part of CS project specs, but there's room for improvement on Primer Spec (see #130, #129). Given that Primer Spec is heavily influenced by GitHub (and even uses GitHub's design system under the hood), it only makes sense to take inspiration from GitHub's components for displaying files containing code.

This PR lets Primer Spec enhance most code blocks on a page with additional features.

Enhanced Code Block Features

  • Line numbers!
  • "Copy" button copies entire code block
  • Clicking the line-numbers selects lines. Click-and-drag to select multiple lines!
  • Lines can be highlighted (as discussed in Highlight line in code block #130).

Limitations

  • Code blocks are only enhanced if they are formatted by Rouge via Jekyll. Regular <pre> and <code> HTML blocks will NOT be enhanced.
  • "Copy" hoverable button only appears for code blocks that are at least 2 lines long.
  • Enhanced code blocks are not optimized for mobile and touch devices [yet]. But their current usability is no worse than the "legacy" code block.

Screenshots

Iteration 1: Code blocks have headers
Default Light Default Dark
Basic usage
Custom title/filename
Highlighted lines
Default Dark
Selecting lines
Screen.Recording.2021-09-28.at.10.52.58.PM.mov
Selecting lines in `console` code blocks (only the command is selected!)
Screen.Recording.2021-09-28.at.10.54.49.PM.mov
Copy entire Gist
Screen.Recording.2021-09-28.at.10.55.43.PM.mov
Iteration 2: GitHub README style (Latest)
Basic Usage
Selecting lines
Screen.Recording.2021-10-01.at.11.37.43.PM.mov
Copying entire code block
Screen.Recording.2021-10-01.at.11.40.16.PM.mov

Demo & Validation

Visit https://preview.sesh.rs/previews/eecs485staff/primer-spec/131/demo/enhanced-code-blocks.html to interact with the demo yourself!

Acknowledgements

Thanks @bellakiminsun for brainstorming what this feature could look like (and other ideas for Primer Spec :) )
Thanks @awdeorio for your feedback and enthusiasm in #129 and #130!
Thanks GitHub for making your design system open-source 😅

@seshrs seshrs added enhancement New feature or request semver/minor Pull Request proposes "minor" change labels Sep 29, 2021
@seshrs seshrs added this to the WN 2022 milestone Sep 29, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2021

The spec from this PR is available at https://preview.sesh.rs/previews/eecs485staff/primer-spec/131/.

(Available until Mon Nov 08 2021.)

@awdeorio
Copy link
Contributor

awdeorio commented Sep 29, 2021

This is so great!

The name "gist" is slightly confusing. This new feature has some things in common with GitHub Gists like the pretty formatting, and the philosophy of sharing snippets of code. This new feature also has some things in common with Markdown code blocks, namely the syntax. What do you think about a name like "smart code block" or "interactive code block"?

Line numbers!

Great idea. At first I thought that they might be confusing for console blocks, but after some reflection, I think that they are really useful for discussing the console blocks with students. E.g., "tell me where your output is different from the tutorial".

"Copy" button copies entire code block

I think selecting the entire thing is exactly the right move here.

Clicking the line-numbers selects lines. Click-and-drag to select multiple lines!
When the language is console, clicking a line number selects just the command, not the prompt. (CC @awdeorio, this is what we discussed in Copy code block #129 (comment)!)

OMG I've been waiting for this and didn't even know it. The $ symbol is essential for understanding a console block, but it's annoying for copy/paste.

Clicking the line number to highlight was slightly unintuitive for me. Here's a possible alternative: mouse over a line displays a copy-paste icon. Click the icon to copy one line. Popup says "Copied!" just like the "copy entire code block" feature. Possibly click anywhere on the line to copy, not just on the icon. I don't consider this a dealbreaker, just one person's feedback.

Lines can be highlighted (as discussed in Highlight line in code block #130).

Very cool!

Gists can have titles. Default title is the code block's language.

What do you think about a default of no title? Optionally add a title as currently implemented. The reason I say this is a title of console or python isn't really very helpful, but it's prominent in the UI.

Additionally, the title of the code block is a link to an HTML fragment. This syntax is inconsistent with the rest of Primer Spec, which uses a linkify icon next to each title.

I know that I left a bunch of feedback, but please consider this an excited and positive sentiment!

@seshrs
Copy link
Member Author

seshrs commented Sep 30, 2021

I know that I left a bunch of feedback, but please consider this an excited and positive sentiment!

Thanks so much for your feedback @awdeorio! Your enthusiasm and feedback really mean a lot :)

OMG I've been waiting for this and didn't even know it. The $ symbol is essential for understanding a console block, but it's annoying for copy/paste.

😁

The name "gist" is slightly confusing.

Agreed, and @bellakiminsun gave me the same feedback lol. We discussed how maybe this should just be the default code block experience for all Primer Spec pages (unless the code block is less than 3 lines long, in which case line numbers feel like overkill). Maybe we display line numbers ONLY if there are more than 2 lines...? (Still thinking out loud here.)

Clicking the line number to highlight was slightly unintuitive for me.

While I understand where you're coming from, I would personally expect "clicking the margin to select a line" to be an "established" UX. I see this behavior on GitHub diff reviews, VSCode, Facebook's internal source control tools, and even Microsoft Word!

I also think it could be useful for students to quickly copy a subset of lines rather than just a single line or the entire block. I'm not 100% sure what a line-by-line copy button would look like, I would need to think about this a bit more.

What do you think about a default of no title?
Additionally, the title of the code block is a link to an HTML fragment. This syntax is inconsistent with the rest of Primer Spec, which uses a linkify icon next to each title.

That's a great point about inconsistency with the rest of Primer Spec! I wonder where the linkify icon would appear in a gist if it had no title... need to think about this 🤔

I kind-of assumed that knowing the language of a code block can help provide context when no title is provided, but I do agree that it's not super-useful. Do you have suggestions for what else might be useful to see in the header section of the code block?
Alternatively, maybe there shouldn't even be a header, but we'd have to put the "copy" button somewhere else.

@awdeorio
Copy link
Contributor

awdeorio commented Oct 1, 2021

The name "gist" is slightly confusing.

Agreed, and @bellakiminsun gave me the same feedback lol. We discussed how maybe this should just be the default code block experience for all Primer Spec pages (unless the code block is less than 3 lines long, in which case line numbers feel like overkill). Maybe we display line numbers ONLY if there are more than 2 lines...? (Still thinking out loud here.)

A simple and reasonable solution in my opinion would be to make this new approach the default for all Primer Spec code blocks. Users could optionally disable the enhanced code block, which would result in a default GitHub Flavored Markdown code block.

That's a great point about inconsistency with the rest of Primer Spec! I wonder where the linkify icon would appear in a gist if it had no title... need to think about this 🤔

How about being consistent with upstream? Example in my comment on the issue #129 (comment)

I kind-of assumed that knowing the language of a code block can help provide context when no title is provided, but I do agree that it's not super-useful. Do you have suggestions for what else might be useful to see in the header section of the code block?

I would be in favor of removing the header by default. A directive could enable the optional header. Here's just more ideas: an optional filename could make it easy to download the entire example to a predetermined filename.

@seshrs seshrs changed the title [Feature] Gists: Enhanced code blocks [Feature] Enhanced code blocks Oct 2, 2021
Copy link
Member Author

@seshrs seshrs left a comment

Choose a reason for hiding this comment

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

One thing to consider: This feature currently only enhances code blocks that were transformed using Jekyll+Rouge.

I wonder how hard it would be to enhance any arbitrary <pre> block…

Edit: Done in f158082

demo/enhanced-code-blocks.md Outdated Show resolved Hide resolved
demo/enhanced-code-blocks.md Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver/minor Pull Request proposes "minor" change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants