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

Fix blockquote rendering #923

Merged
merged 1 commit into from
Oct 1, 2019
Merged

Fix blockquote rendering #923

merged 1 commit into from
Oct 1, 2019

Conversation

johnSchnake
Copy link
Contributor

Will not be linked to the docs site but will be accessible by URL.
Just useful for devs who may have questions about format.

@johnSchnake
Copy link
Contributor Author

From the page https://deploy-preview-923--sonobuoy.netlify.com/docs/master/markdown-cheatsheet/

We can see problems with:

  • underlined header styles
  • tables
  • syntax highlighting in code blocks
  • blockquotes
  • some issue (not sure how much) with inline html

@codecov-io
Copy link

codecov-io commented Sep 30, 2019

Codecov Report

Merging #923 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #923   +/-   ##
=======================================
  Coverage   47.59%   47.59%           
=======================================
  Files          76       76           
  Lines        5175     5175           
=======================================
  Hits         2463     2463           
  Misses       2562     2562           
  Partials      150      150

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ba8b74...a304cc4. Read the comment docs.

@johnSchnake
Copy link
Contributor Author

Trying to change the markdown processor from redcarpet to kramdown did not fix this. Both syntax seem to support the same blockquote syntax (> text) but neither is working so I'm assuming it is something else higher in the stack.

That led me to look at the source html and there is actually a <blockquote> tag, so the css must be involved in removing the look we want. Taking a look...

@johnSchnake
Copy link
Contributor Author

I can see that it doesn't seem to have the border-left that I expect but adding it doesn't seem to have the effect I intend. I also tried to toggle lots of css off to see if it would look as expected, but I can't seem to track it down.

@johnSchnake
Copy link
Contributor Author

See https://deploy-preview-923--sonobuoy.netlify.com/docs/master/markdown-cheatsheet/#blockquotes for the effect.

It was just had some css for the blockquotes that didnt have the borders set

@johnSchnake
Copy link
Contributor Author

If you are opposed to the file let me know; I can remove it. Having it go forward would just make it easier to check our rendering/css. It came from https://github.com/adam-p/markdown-here; I'm just copying a file of sample markdown so I'm not under the impression that I have to cite the MIT license due to the size/value of the data. If you disagree I can add another custom one that just spits out that sort of stuff.

@johnSchnake johnSchnake changed the title Adding a markdown test file for rendering on docs site. Adding a markdown test file for rendering on docs site Sep 30, 2019
@johnSchnake johnSchnake changed the title Adding a markdown test file for rendering on docs site Fix blockquote rendering Sep 30, 2019
@johnSchnake
Copy link
Contributor Author

Done tweaking and happy with the look now (on the sample page and throughout other docs). Feel free to review now.

Copy link
Contributor

@zubron zubron left a comment

Choose a reason for hiding this comment

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

I'm happy for the file to be added and think the styles look good. I would have thought that bootstrap provided better styling for blockquotes, but oh well :) I'm not sure if there's a different way to provide these styles rather than modifying the vendored files. Perhaps @jonasrosland will have a suggestion for that. LGTM though 👍

@johnSchnake
Copy link
Contributor Author

johnSchnake commented Oct 1, 2019

Yea it seems bootstrap just doesn't do much with the blockquote: https://getbootstrap.com/docs/4.0/content/typography/#blockquotes

I didnt even realize when I jumped to those files they were vendored at first. That isn't a great approach since the changes are easily lost. Wait now I'm confused. I thought I had looked and seen they were in vendor but now I see it is just in the site code which is a bit different.

Maybe I should just adjust the PR to make all those note blockquotes italic'd or something.

@jonasrosland let me know what you think is best. Either way I just want to clean up that formatting.

Fixes problem with blockquotes which we utilize often throughout docs.

Fixes #918

Signed-off-by: John Schnake <[email protected]>
@johnSchnake
Copy link
Contributor Author

Ended up removing the cheatsheet; since we have the netlify preview it shouldn't be a big deal really. Having the cheatsheet wouldnt have helped unless we know what to look for in the first place.

Having the custom css doc will make it going forward if we want to tweak things.

@johnSchnake johnSchnake requested a review from zubron October 1, 2019 20:43
@johnSchnake johnSchnake merged commit c799770 into vmware-tanzu:master Oct 1, 2019
@johnSchnake johnSchnake deleted the jekyllBlockquotes branch October 1, 2019 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants