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

Codeblock refactor w/ hide/show [Fixes #1776] #2066

Merged
merged 11 commits into from
Dec 8, 2020
Merged

Codeblock refactor w/ hide/show [Fixes #1776] #2066

merged 11 commits into from
Dec 8, 2020

Conversation

wackerow
Copy link
Member

@wackerow wackerow commented Dec 6, 2020

Description

Refactors codeblocks, automatically collapsing code blocks over 8 lines, with button to show all/show less, and refactored Copy button to address color dissonance and to match new changes.

  • Moved buttons to float over upper right corner of codeblock
  • show all/show less button toggles between collapsed with overflow scrolling, and showing all lines of code
  • padding-top adjusted IF buttons present, to avoid overlapping for smaller devices or long strings on line 1 of code, leaving simple codeblocks without copy functionality grossly unchanged
  • Both buttons given unobtrusive hover-over effects
  • Changes coloring to always work on the current codeblock coloring theme
  • Decreased font-size by 25% for button text
  • Added logic to remove last line if blank (per a TODO comment)
    (On that note, went through the markdown files and removed any blank lines that were placed before closing ``` tag, which were adding an additional empty line at the end of codeblocks when rendered)
  • Logic hides show/hide button if codeblock 8 lines or less.
  • Logic for showing/hiding copy button remains unchanged (only shows for supported languages)

Related Issue #1776

(*autoformatting added a \ before $USD on oracles page... two commits trying to fix it just keeps autoformatting it back in)

wackerow and others added 7 commits December 6, 2020 09:52
reverted Icon.js changes
placed this value into its own constant to help with redundancy. removed unused import. eliminates large padding-top if no 'copy' nor 'hide/show' buttons being displayed
@github-actions github-actions bot added Status: Review Needed content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels Dec 6, 2020
Copy link
Contributor

@ryancreatescopy ryancreatescopy left a comment

Choose a reason for hiding this comment

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

Thanks Paul, my only comment is on the button size. I understand why you've made the text smaller but I think this could be at the detriment of accessibility – it's pretty hard to read. I think it'd be ok with the button copy at 16px as per the others.

@wackerow
Copy link
Member Author

wackerow commented Dec 7, 2020

image
That is with default 1rem size... and yeah, I agree, this size looks clean still and easier to read.

@ryancreatescopy That padding look okay to you? Let me know if you have any other design suggestions, happy to tweak as needed.

Copy link
Contributor

@ryancreatescopy ryancreatescopy left a comment

Choose a reason for hiding this comment

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

Looks good to merge!

@ryancreatescopy ryancreatescopy merged commit 4044b46 into ethereum:dev Dec 8, 2020
@wackerow wackerow deleted the w/codeblock#1776 branch December 8, 2020 16:55
@samajammin samajammin mentioned this pull request Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants