This PR does two things:
1. fixes using mermaid version `>= 10` from the CDN, by importing the ESM module instead
2. moves script loading code from `head.html` to the mermaid include
I've also added some light documentation to clarify how using mermaid with local paths should work (users should specify a version, and they should only use fully-minified bundles with no local references).
The nice thing about this approach is that it's a breaking change for nobody, and only adds functionality (v10 support). Eventually, we should remove support for mermaid <10, which should make this much easier!
Closes#1206.
## Context
In v10, Mermaid has implemented a few (admittedly, very frustrating to deal with) breaking changes:
1. they've removed CJS support, which is fine, *but*
2. that means that the `dist` they publish to JSDelivr now has a **different URL**: for versions `10.0.0` - `10.0.2`, **they do not have a minified bundle -- you have to load the ESM version with relative imports**
3. and, separately the `init` function has been deprecated
2 is really the issue, and so I've had to go into the code to now load mermaid by ESM by default when the user is on mermaid > v10.
I've tested this with:
- CDN version < 10 (v9)
- CDN version 10
- local path with version < 10 (v9)
- local path with version 10 (new: also loaded as an ESM module)
Separately, I chose to put all the mermaid stuff in one include because:
- I think @pdmosses requested something like this - it's a bit confusing that some mermaid code is *not* in the include, and this makes modular components ... more modular
- from a developer perspective, it's more clear what's happening with mermaid
- mermaid is not render-blocking, so it shouldn't be in the `head` anyways
---------
Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
The scope of this PR has changed slightly - it now removes all styling of `::selection`, which reverts selected-element highlighting to browser defaults (typically a blue highlight with no text colour changes). It still inadvertently closes#1201.
I've included the original PR body below:
---
This resolves an issue on Firefox where selecting a code block produces white text on a white background, which is not legible. To test: visit https://deploy-preview-1208--just-the-docs.netlify.app/docs/index-test/, and highlight various code blocks in light/dark mode.
I did a bit more digging, and realized a handful of things:
- when I added the new `OneLightJekyll` theme, I inadvertently bundled in a `::selection` class; I've removed it.
- I'm not really sure why this is a part of the theme in the first place!
- this is technically the minimum change required to have no more issues
- however, at this point, Firefox now correctly uses the global `::selection`, which is white-on-purple; this is *different* from Chrome, which somehow overrides this for `pre` or `code`; I also (subjectively) think this is harder to read.
- the vast majority of websites default to the browser/user agent stylesheet for code highlighting; for example, [react.dev](https://react.dev)
- so, I've elected to instead default to the browser/user agent stylesheet; this has the nice side effect of making Chrome and Firefox consistent again
Questions for reviewers/community members:
- does this fix the problem for you? what about other browsers?
- do we like having the browser default for code selection, or should we stick to white-on-purple?
Closes#1201.
Noticed that we have some images in `assets/` that aren't linked-to from any part of the site. `just-the-docs.png` is used as an example for enabling an image header, but it's not actually called-to. `search.svg` doesn't exist at all (it was moved to an icon, but the original file was never deleted).
In the conversation for #1166, I noticed that the import order for `setup.scss` disagrees with our docs.
> In particular, the [docs for `setup.scss`](https://just-the-docs.com/docs/customization/#override-and-define-new-variables) reads:
>
> > To define new SCSS variables, functions, or override existing theme variables, place SCSS code in `_sass/custom/setup.scss`
>
> But, this is not true - `setup.scss` is loaded *before* all of the themes, so it doesn't override existing theme variables.
>
> In my opinion, the solution here is to move `setup.scss` after all the `color_scheme` SCSS. This way, `setup.scss` properly allows theme overrides. In addition, users who previously defined variables in `setup.scss` and then used them in their custom color schemes can shift those declarations to be entirely in the theme code.
This is a one-liner that fixes the behaviour to be in-line with what our docs state.
This is a deceptively simple PR that stops the double import of `color_schemes`. With @pdmosses' stellar suggestion, it's a simple two-liner!
## interaction with #1166
This is a clean merge!
## path forward for default syntax highlighting
However, this leaves an interesting question: if the user doesn't provide syntax highlighting as part of their color scheme, should we include a default set (in this case, the light theme)?
Broadly, I see a few arguments:
1. if we don't provide defaults, we'll break color schemes that don't define their own syntax highlighting
2. if we do provide defaults, we could unnecessarily bloat the file size
I think 1 is more pernicious than 2. Thus, my suggested path forward is:
- for now, merge this
- in `v1.0`, separate these concerns properly, and force each color scheme to provide its own syntax highlighting CSS. Provide a default file for users to import with `@import` or `@use`[^1].
[^1]: Separately, we're using `!default` wrong; [looking at the docs](https://sass-lang.com/documentation/variables#default-values), we need to be using it with `@use` for the defaults to take effect. Since we're not doing that, `!default` isn't actually doing anything! This is why variable overrides aren't propagating the way they should be (and thus, users need to do a lot of duplication). Fixing this is probably a v1 item, though I'll have to think about it more.
This PR replaces the default light code highlighting theme with Atom's One Light theme colors. This should provide more visual similarity with our dark theme, and as a byproduct, fix some of the contrast issues from our current light theme.
In addition (different from the original purpose of this PR), this also moves theme variables from `variables.scss` to `light.scss`, which always gets loaded anyways.
To test, compare the [deploy preview's kitchen sink Python code](https://deploy-preview-1166--just-the-docs.netlify.app/docs/index-test/#more-code) to the current [`main` branch's code](https://just-the-docs.github.io/just-the-docs/docs/index-test/#more-code); you can also use the "Preview dark color scheme" button to see OneDarkJekyll in action.
Users can opt-in to the old theme with `legacy_light`. I've documented this in the "customization" page.
Closes#679.
## implementation
Feel free to skip this part.
To do this, I:
- forked [mgyongyosi/OneDarkJekyll](https://github.com/mgyongyosi/OneDarkJekyll) to our own organization, [OneLightJekyll](https://github.com/just-the-docs/OneLightJekyll)
- updated the code to be slightly more robust (e.g. not require installing to global path)
- replaced the `colors.less` with the one pulled from Atom's [one-light-syntax](https://github.com/atom/atom/tree/master/packages/one-light-syntax)
- updated the license notice to also include GitHub's work in Atom
- regenerated the file
- plopped it in our current theme code
## next steps
This is related to #1100. I wanted to make this PR easier to review, so I won't implement that just yet; once we merge this, I can push that PR through. It's also related to #1173; in a comment below, I've also outlined some potential future work.
After we merge this, I'll trigger a release soon. I think this next minor bump can be focused on color schemes, e.g. dark theme, theme switching, and some bugfixes.
As discussed in #1174, this adds a documentation section to UI Components > Code > Mermaid that describes the usage of mermaid with AsciiDoc.
Regarding the comment on asciidoctor-diagram in my edits, I cannot stress enough how much pain this is to set up (this was the first thing I tried before switching to JTD's client-side mermaid support). It basically pre-renders mermaid diagrams in a headless chromium browser. This requires manual configuration of Puppeteer, along with additional config for Jekyll to keep the images the extension creates. And when you managed to get the image on your site, it looks horrible. This is why I wrote „not recommended“.
I introduced a bug in #1172 --- there's an implicit requirement that the line in `mermaid_config.js` has either an expression or the start of a code block; a comment (and then a newline) is invalid.
(to maintain the ignore behaviour, I've instead moved it to the `.prettierignore`)
This one's on me -- I hadn't realized that the `prettier-ignore` affected this file. Given the relative urgency of this, going to merge this in.
Closes#1188.
Short and sweet PR that does exactly what the title says. Note that this only affects developers of our theme, and is a no-op on the rest of the codebase (including formatting) -- it's just moving a config file!
The nice benefits: higher precedence, and one less file (that can confuse users who are forking this repo).
This is an internal change that only affects developers of our theme. This PR:
- **upgrades `Stylelint` to `v15`!**
- among other things, this leads to the deprecation of many stylistic rules, which overlap with Prettier
- it also sees the addition of new rules that catch more errors, especially `declaration-property-value-no-unknown`!
- upgrades the accompanying standard config (the SCSS one extends the first-party one); this new config disables the deprecated rules that overlap with Prettier
- removes a now-unneeded plugin that disables overlapping rules
- moves the `stylelint` config object to `package.json` - now we have one less file (particularly important since people still clone this repo)
There were very few new changes; I combined one string that had a dangling `+` in an SCSS warning, and have temporarily disabled the `at-rule-empty-line-before` rule (which also picks up on SCSS `@includes`).
After some discussion in #1166, I realized that our prettier workflow is somewhat broken! This PR fixes it, by:
- separating Stylelint & Prettier (and removing `stylelint-prettier`) to clearly separate linting versus formatting
- this is also helpful for the upcoming upgrading of Stylelint
- having Prettier ignore `assets/js/zzzz-search-data.json`; on `main`, `npm run format` fails because of this file (Prettier doesn't know how to interpret the front matter)
- change `npm test` to run both Stylelint and Prettier on CI (uses `npm-run-all` utility
- had to add one `// prettier-ignore` in `_includes/mermaid_config.js`, which I think is reasonably placed
- removing an unused `script` (that doesn't work!)
This is a no-op on the site itself, just for our dev environment/CI.
Ref: https://github.com/just-the-docs/just-the-docs/pull/1166#discussion_r1110397592.
---
Separately, upgrading to Stylelint 15 will come with another interaction - the [deprecation of stylistic rules](https://stylelint.io/migration-guide/to-15#deprecated-stylistic-rules) + shift to Prettier should be nice!
These changes accommodate for some difference in the HTML structure AsciiDoc produces for source code listings:
* proper padding for source code listings
* proper vertical margin after source code listings
* proper placement of the copy button if enabled
Closes#1163
In my other cleaning, I also noticed:
- the contributors pane on the homepage is broken
- the following link to a repo page in the search docs is broken
<img width="762" alt="Screenshot 2023-01-03 at 6 41 49 PM" src="https://user-images.githubusercontent.com/14893287/210474679-bcb9af23-0a4e-4999-a0ec-f06d967ea726.png">
In particular, the generated HTML is
```html
<a href="/blob/main/assets/js/zzzz-search-data.json">this content</a>
```
Looking at the source code for both features
```html
<ul class="list-style-none">
{% for contributor in site.github.contributors %}
<li class="d-inline-block mr-1">
<a href="{{ contributor.html_url }}"><img src="{{ contributor.avatar_url }}" width="32" height="32" alt="{{ contributor.login }}"></a>
</li>
{% endfor %}
</ul>
```
```md
[this content]({{ site.github.repository_url }}/blob/main/assets/js/zzzz-search-data.json).
```
It's clear that `site.github` is not being populated. This is controlled by the GitHub Metadata/[`jekyll-github-metadata`](http://jekyll.github.io/github-metadata/) plugin.
I'm not when this stopped working. If I had to guess, I think this is packaged as part of the `github-pages` gem; so, when the site moved off of it a while back, we never noticed this regression. This is the type of thing that can hopefully be caught by regression tests in the future.
This PR re-adds the plugin. I've opted to only add it to the `Gemfile` but not the `gemspec` so that it only affects our site. In other words, JtD does not have `jekyll-github-metadata` as a runtime dependency (since none of our theme code relies on it). Happy to change that if we'd like.
---
In the future,
- short-term: I can write a filter that removes dependabot from our contributors
- longer-term: we could rewrite the "last edited on GitHub" feature to instead use `jekyll-github-metadata`. this would necessitate us to make it a runtime dependency, and it also wouldn't work as well for users of GitLab or other alternatives.
This PR:
- matricizes (?) the OS parameter: we now also test on `windows` and `macOS`
- matricizes the Ruby version parameter: we now also test on Ruby 2.7
- loosens the `.gemspec` requirement for `bundler` to allow things greater than `2.3.5`
- interestingly, this is because the Ruby 2.7 container has a **later** version of bundler than the 3.1 one!
The last point makes this user-facing, and thus I'll trigger a release for this PR sometime soon.
These only apply to the `jekyll-build` job. Here, I think we have a broader obligation to support various use-cases (maybe they're deploying on GitLab, generating files locally, etc.); in contrast, the `github-pages` gem is used exclusively for deploying to GitHub Pages, so we should try to match that environment exactly.
This PR closes#1145.
This is a small PR that:
- adds a `fixtures/Gemfile-github-pages`, the minimum gemfile necessary to build the site
- adds a new Actions workflow to build the site with that gemfile
In the future, this should prevent issues like #1074, where we push a change that passes the `jekyll` CI but fails to build on `github-pages` (and its dependencies - which in this case, was an older version of ruby sass).
(to be honest - not super happy with the code duplication, but I also think we'll have different build matrices for the different versions, so I'm fine with this for now)
Part of (but does not close) #1145.
