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 workaround with GitHub latest styles #28

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Fix workaround with GitHub latest styles #28

merged 2 commits into from
Dec 11, 2023

Conversation

hyrious
Copy link
Collaborator

@hyrious hyrious commented Dec 11, 2023

This PR did various changes to make it work with the latest styles from GitHub.

  • Fixed an oversight in previous versions where auto.css includes duplicated CSS variables. See the light section here: https://cdn.jsdelivr.net/npm/github-markdown-css@5.4.0/github-markdown.css
  • Refactored works from Add support for alerts #26 to match the latest alert structures:
    <div class="markdown-alert markdown-alert-note" dir="auto">
      <p class="markdown-alert-title" dir="auto">
        <svg class="octicon octicon-info mr-2" viewBox="0 0 16 16" version="1.1" width="16" height="16" aria-hidden="true"><path d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8Zm8-6.5a6.5 6.5 0 1 0 0 13 6.5 6.5 0 0 0 0-13ZM6.5 7.75A.75.75 0 0 1 7.25 7h1a.75.75 0 0 1 .75.75v2.75h.25a.75.75 0 0 1 0 1.5h-2a.75.75 0 0 1 0-1.5h.25v-2h-.25a.75.75 0 0 1-.75-.75ZM8 6a1 1 0 1 1 0-2 1 1 0 0 1 0 2Z"></path></svg>
        Note
      </p>
      <p dir="auto">Highlights information that users should take into account, even when skimming.</p>
    </div>
    Which means it now adds classes like markdown-alert-note markdown-alert-title octicon-info mr-2 to the ALLOW_CLASS set.
  • Refactored ALLOW_TAGS to match the latest sanitizer's code.
  • Workaround new CSS syntax @container to make it able to parse GitHub CSS again.

To read the output diff:

$ node test
$ wget https://cdn.jsdelivr.net/npm/github-markdown-css/github-markdown.css
$ git diff --no-index github-markdown.css dist/auto.css

Next steps:

As I mentioned in #27, GitHub will use newer CSS syntaxes from now on -- they are using @container, maybe the next is @layer or even CSS nestings. We need not a workaround but a solution to parse these new CSS syntaxes. Maybe integrating lightningcss, or making a fork of the css module to add supports (it is not updated for 2 years).

cli.js Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Maybe integrating lightningcss, or making a fork of the css module to add supports (it is not updated for 2 years).

I think we should switch. CSS doesn't stop evolving, so it doesn't make sense to continue maintenance of an abandoned package.

@sindresorhus sindresorhus merged commit 7e7b73c into main Dec 11, 2023
4 checks passed
@sindresorhus sindresorhus deleted the fix-parse branch December 11, 2023 14:09
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.

2 participants