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

@patrickhlauke patrickhlauke added this to Inbox in v5.0.0-alpha2 via automation Jun 21, 2020
@patrickhlauke patrickhlauke added this to Inbox in v4.5.1 via automation Jun 21, 2020
@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!

@mdo mdo added this to In progress in v4.5.3 via automation Aug 4, 2020
@mdo mdo removed this from Inbox in v4.5.1 Aug 4, 2020
@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

v5.0.0-alpha2 automation moved this from Inbox to Approved Sep 1, 2020
@patrickhlauke patrickhlauke merged commit 0e007e6 into main Sep 2, 2020
v5.0.0-alpha2 automation moved this from Approved to Shipped Sep 2, 2020
@patrickhlauke patrickhlauke deleted the docs-highlight-pre-tweak branch September 2, 2020 06:17
@XhmikosR XhmikosR moved this from Inbox to Shipped in v4.5.3 Sep 10, 2020
@XhmikosR XhmikosR moved this from Shipped to Needs manual backport in v4.5.3 Sep 17, 2020
@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
@XhmikosR XhmikosR moved this from Needs manual backport to Shipped in v4.5.3 Sep 19, 2020
@XhmikosR XhmikosR removed this from Shipped in v4.5.3 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 <xhmikosr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5.0.0-alpha2
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

4 participants