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

Docs: Add top/bottom margin to highlighted code samples #31036

Merged
merged 13 commits into from
Sep 2, 2020

Conversation

patrickhlauke
Copy link
Member

@patrickhlauke patrickhlauke commented Jun 15, 2020

provide more space to avoid having the "Copy" button overlap code sample content

Preview: https://deploy-preview-31036--twbs-bootstrap.netlify.app/

Before:

highlight-pre-1

highlight-pre-2

After:

highlight-pre-1-after

highlight-pre-2-after

provide more space to avoid having the "Copy" button overlap code sample content
@patrickhlauke patrickhlauke requested a review from a team as a code owner June 15, 2020 10:39
@patrickhlauke
Copy link
Member Author

admittedly looks a bit "spacey" for code sample one-liners, but works well for content that is very long/wide like in the second example (where otherwise you get the unsightly overlap of "Copy")

@ffoodd
Copy link
Member

ffoodd commented Jun 15, 2020

I'm definetely for this. If spacing doesn't look good, might we consider adding a background-color?

@patrickhlauke
Copy link
Member Author

there is a subtle background color already. what i meant more was that it looks odd being top-right in an otherwise one-liner box (where currently for single-line code blocks like that, the "Copy" looks inline with the single line of code)

@patrickhlauke
Copy link
Member Author

or did you mean a background on the non-hovered/non-focused "Copy" button?

@ffoodd
Copy link
Member

ffoodd commented Jun 15, 2020

Indeed, a background for the copy button: to make it stand out a bit, and prevent it from being unreadable.

Note sure it'd be better than spacing though.

@mdo mdo changed the base branch from master to main June 16, 2020 19:29
@twbs twbs deleted a comment from patrickhlauke Jun 17, 2020
@patrickhlauke
Copy link
Member Author

patrickhlauke commented Jun 17, 2020

while not obvious anymore on the new v5 homepage (as the code samples have been prefixed with a comment that's short and doesn't creep to the right-hand side), the fundamental problem is still there in places, e.g. https://v5.getbootstrap.com/docs/5.0/forms/form-control/#sizing

screenshot of v5 docs with copy button covering code

@mdo
Copy link
Member

mdo commented Jul 20, 2020

On the Icons site, I've changed the button style vs adding more vertical margin/padding. See https://icons.getbootstrap.com/icons/alarm/ for an example. I'd be more in favor of that vs the added page height.

@patrickhlauke
Copy link
Member Author

@mdo it will still overlap in those situations...but will look prettier. up to you.

@patrickhlauke
Copy link
Member Author

@mdo taking the example for v5 from #31036 (comment) and grafting in the styles for the clipboard buttons from https://icons.getbootstrap.com/icons/alarm/ still results in the buttons then covering/overlapping the content. true, it's prettier. but functionally still the same...they get in the way.

example from v5 with the prettier clipboard buttons styles from icons site - still overlap actual content

how about combining the two approaches? nicer styles for the clipboard buttons and more padding?

@mdo
Copy link
Member

mdo commented Aug 3, 2020

how about combining the two approaches? nicer styles for the clipboard buttons and more padding?

Let's try it out!

@patrickhlauke
Copy link
Member Author

added new styles for the clipboard button, and tweaked both the size/position of the button and my added margin ... to some middle-of-the-road compromise. still overlaps content a bit if the first line fills the whole width, but at least doesn't look broken now. and the slightly reduced size of the clipboard button feels good, subjectively.

screenshot of latest change

@patrickhlauke patrickhlauke requested a review from mdo August 19, 2020 22:31
@patrickhlauke
Copy link
Member Author

not urgent, but just to clear the deck of relatively simple little things... ping @mdo @twbs/css-review

@patrickhlauke patrickhlauke merged commit 0e007e6 into main Sep 2, 2020
@patrickhlauke patrickhlauke deleted the docs-highlight-pre-tweak branch September 2, 2020 06:17
@XhmikosR
Copy link
Member

@patrickhlauke can you please backport this in v4-dev?

@patrickhlauke
Copy link
Member Author

@XhmikosR done #31706

XhmikosR pushed a commit that referenced this pull request Sep 19, 2020
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
* Add top/bottom margin to highlighted code samples

provide more space to avoid having the "Copy" button overlap code sample content

* Modify clipboard button style

Per twbs#31036 (comment) and twbs#31036 (comment)

* Tweak margin, clipboard button size and position

Co-authored-by: XhmikosR <[email protected]>
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.

4 participants