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

Recent Block Code Highlighting Style Issues #1451

Closed
4 tasks done
facelessuser opened this issue Feb 8, 2020 · 18 comments
Closed
4 tasks done

Recent Block Code Highlighting Style Issues #1451

facelessuser opened this issue Feb 8, 2020 · 18 comments
Labels
bug Issue reports a bug

Comments

@facelessuser
Copy link
Contributor

I checked that...

  • ... the documentation does not mention anything about my problem
  • ... the problem doesn't occur with the default MkDocs template
  • ... the problem is not in any of my customizations (CSS, JS, template)
  • ... there are no open or closed issues that are related to my problem

Description

Small highlight issues when dealing with code blocks in tabbed interfaces and in admonitions.

I suspect part of the problem has to do with some of the mobile logic being removed in the last bugfix.

  1. non table code blocks have padding below them when they are the only element in a
    tabbed interface. This is not an issue with the tabbed code blocks with line numbers.

  2. When in mobile interface, the code block spills out of the tabbed interface and extends to the width of the admonition block.

Below we see both issues in the first code example. There seems to be no issues in the tabled version.

firefox_EWpggBUTCm

Expected behavior

  1. I'd expect the code blocks to extend to the full length of the tabbed interface like they use to.

  2. In mobile interfaces, code blocks would extend properly to the width of the tabbed interface.

Actual behavior

The opposite of expected. See picture above.

Steps to reproduce the bug

  1. View a single code block in a tabbed interface for first issue.

  2. View a code block in a tabbed interface within an admonition for the second issue.

Package versions

  • Python: Any
  • MkDocs: Any
  • Material: 4.6.1

Project configuration

System information

  • OS: Any
  • Browser: Any
@squidfunk squidfunk added the bug Issue reports a bug label Feb 8, 2020
@squidfunk
Copy link
Owner

squidfunk commented Feb 8, 2020

Thanks for reporting those cases. There was a lot of hacking for making code blocks to work in all possible circumstances (especially due to the inconsistencies with .codehilite > code), so there's a lot of possibility for cleanup. I'll fix those cases and try to clean up as much as I can, but I'll definitely revisit it for v5.

@squidfunk
Copy link
Owner

Should be fixed in master with c177506. The issues were mainly related to inconsistent margin/padding definitions. I tested without any highlighting, with CodeHilite and Highlight on the top-level, inside tabbed code blocks and inside Admonitions and hope that I catched all cases. Could you verify the latest master? I'll issue a bugfix release afterwards.

@squidfunk
Copy link
Owner

... we definitely need to set up some layout testing, but screenshot testing is a real pain. I'm working on a solution as part of a side-project and will use Material as a first test case. Changing CSS with confidence is a really hard problem.

@facelessuser
Copy link
Contributor Author

Thanks for looking into to this, and I totally understand. When the new release dropped, I ran it through its paces because I kind of expected something to slip by. Verifying complex CSS with so many little corner cases is kind of a pain.

I'll verify master and get back to you.

@facelessuser
Copy link
Contributor Author

Looks great 👍 .

I can finally kick the codehilite class too!

@squidfunk
Copy link
Owner

Perfect, thanks for your help! Released as part of 4.6.2.

@wilhelmer
Copy link
Contributor

In c177506 , you removed the padding for .superfences-content pre in _superfences.scss. Now my superfences containers look like this:

superfences-padding

Reintroducing the padding fixes it for me without any noticeable side effects.

// Actual content
pre {
   padding: px2rem(10.5px) px2rem(16px);
   margin: 0;
   border-radius: 0
}

@squidfunk
Copy link
Owner

squidfunk commented Feb 11, 2020

The anomaly you're describing is most likely related to stale dependencies. The padding is now consistently defined on code, see #1454 (comment)

@wilhelmer
Copy link
Contributor

Oh yes, you're right. Pygments was out of date and not updated when I did pip install mkdocs-material --upgrade. Had to run pip install pygments --upgrade, now it works.

@wilhelmer
Copy link
Contributor

Anyway, if Material doesn’t render properly anymore without the additional code tags, I think you should increase the minimum required version for pygments to 2.4 in requirements.txt.

Sent with GitHawk

@facelessuser
Copy link
Contributor Author

Yeah, agree that requiring the latest pygments is probably ideal. Pymdown-extensions doesn't force a requirement for pygments as it is optional (some people prefer JS highlighters), but since material does rely on it, it probably should update the requirements.

@squidfunk
Copy link
Owner

As with the problems we had with Markdown and MkDocs, won't pip just ignore that Material demands a newer version of Pygments when not explicitly upgraded? And why Pygments 2.4? Why not 2.6, the latest version?

@facelessuser
Copy link
Contributor Author

It was more a requirement order thing that got us. If the only package requiring Pygments is material, and it doesn't meet the requirement, it should update. Pip's dependency management sucks, but they do have plans to make it better. I don't think mkdocs requires pygments, nor does pymdown-extensions, so it should be okay? But it kind of depends on the user requirement setup. For a package, I always require what I need so it works when my package is explicitly installed via pip. I can't control what other people do in their requirements that messes that up. All you can do is the what makes sense for your package.

You could do >=2.6, but the minimum requirement is >=2.4 as that is the version that Pygments added the option to wrap code with pre and code.

@facelessuser
Copy link
Contributor Author

@squidfunk
Copy link
Owner

Thanks for the explanation. I was skimming through their changlog but didn't see anything related to the pre > code problem for 2.4.

@squidfunk
Copy link
Owner

Upgraded Pygments in 8c44604

@facelessuser
Copy link
Contributor Author

facelessuser commented Feb 11, 2020

It's some new wrap_code option or something like that. Python Markdown was just forcing it to always be enabled, but Pygments doesn't force it by default. Pymdown-extensions wanted to match Python Markdown to at least have a uniform expectation (with a legacy option to give people time to transition if desired). I like the direction, but it hasn't gone without hiccups 🤷‍♂️.

@wilhelmer
Copy link
Contributor

Thanks for the explanation. I was skimming through their changlog but didn't see anything related to the pre > code problem for 2.4.

Waylan mentions v2.4 in the release notes for markdown 3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug
Projects
None yet
Development

No branches or pull requests

3 participants