PrivateBin icon indicating copy to clipboard operation
PrivateBin copied to clipboard

Better presentation of Markdown in Bootstrap5

Open ww7 opened this issue 1 year ago • 9 comments

The problem

Makrdown text merges with the footer and top menu

The solution

Create some spacing (padding) on top and bottom of paste section, add some visual separation with lines or background color.

ww7 avatar Aug 20 '24 21:08 ww7

I retested this and it does have the expected default paragraph spacing between the top and bottom, which I feel is adequate and looks nice (to me). I would therefore suggest that you solve this via a custom template.

Do other maintainers feel differently about this? Otherwise I'll close this as wont-fix.

elrido avatar Aug 31 '24 17:08 elrido

For compare, added padding 2rem; and background-color the same as in header.

ww7 avatar Aug 31 '24 18:08 ww7

Okay, you should have attached such an image directly (for such a visual issue9. IMHO, just looking at the images, the right one looks better to me personally, indeed?

I guess the right one is your proposed solution? If so, it's technically of course more about the spacing to the left and right.. well... anyway. If @elrido agrees, and this is fine for you, it would be great if you could open a PR to get this merged @ww7. Note, please also test (and screenshot) it in mobile screens/responsive test tools etc. to make sure it also works on smaller screens.

Edit: Also it would be useful to see whether the same style should be applied to other formats, like text-only. (Source code likely not, because it has it's own style already.)

rugk avatar Sep 04 '24 22:09 rugk

So, essentially you wanted to have the "card" CSS class get applied to plain text and markdown content, same as it currently is for the source code? https://github.com/PrivateBin/PrivateBin/blob/e865bec9c38bda15ee59d0b29e4463abc50b71ab/tpl/bootstrap5.php#L429-L432

As stated, I do not like this, visually, and would prefer to not display these two content types differently, but yes, if a PR is raised and other maintainers prefer that distinct style as well, I won't stand in the way. These are just personal differences in taste.

elrido avatar Sep 05 '24 04:09 elrido

You can see a difference between presentation of "Plain Text", "Source Code" and "Markdown, where first two showing content with background and padding.

Personally I simply edited common.css

#plaintext {
	padding: 2rem;
	background-color: rgb(248, 249, 250)!important;
}

article {
	margin-bottom: 2rem;
}

This is not a solution for PR for sure.

ww7 avatar Sep 05 '24 10:09 ww7

I am not sure that it is a theme of the issue, but it is anyway related to the bootstrap5 template. I think that we need to add more gap between header and other content as we have on the regular bootstrap template, because the content is too close to the header IMHO. Also icons inside buttons are not centarized, it could be fixed as well. image

image

Ribas160 avatar Jan 05 '25 20:01 Ribas160

The spacing is easy to tweak using the bootstrap provided margin & padding utilities: Adding class mb-3 to the nav-bar for example adds a bottom-margin of 1 rem.

Regarding the button alignment, I found it being reported to upstream in 2021 and a possible solution discussed there, but no indication of it getting fixed. We may in the interim want to apply one of those solutions to our custom CSS, ideally with a comment referring to the issue URL at upstream, so we know we can remove it once it gets fixed and we update bootstrap.

elrido avatar Jan 06 '25 06:01 elrido

@elrido

We may in the interim want to apply one of those solutions to our custom CSS, ideally with a comment referring to the issue URL at upstream, so we know we can remove it once it gets fixed and we update bootstrap.

We can fix it without adding custom css, we can use Bootstrap5 flex and add these classes to buttons d-flex justify-content-center align-items-center gap-1. The only thing I am worrying about is we have to duplicate these classes to all buttons. I'm not a fun of a code duplication, but anyway for now we don't use any component structure inside the layout.

Ribas160 avatar Jan 06 '25 08:01 Ribas160

Well hacking around with adding many bootstrap classes is not really better, though. I've pinged the upstream issue, this is clearly a bootstrap bug and crazy that this has not been fixed yet (I hope it's not us misusing something).

rugk avatar Feb 26 '25 00:02 rugk