Skip to content

feat: Upgrade from Jekyll 3.x to 4.x#90

Merged
smockle merged 3 commits intomainfrom
jekyll-v4
Dec 8, 2020
Merged

feat: Upgrade from Jekyll 3.x to 4.x#90
smockle merged 3 commits intomainfrom
jekyll-v4

Conversation

@smockle
Copy link
Copy Markdown
Contributor

@smockle smockle commented Dec 5, 2020

Rationale

Jekyll 4.x allows Catalyst to use Kramdown 2.3.0, as suggested by CVE 2020-14001.

Jekyll 3.x to 4.x Upgrade Guide

The upgrade guide notes several changes:

🚨 Before Merging 🚨

While testing this branch locally, I’ve noticed the following unexpected behavior, which we should investigate further before merging:

  1. The warning GitHub Metadata: No GitHub API authentication could be found. Some fields may be missing or have incorrect data. is logged when running bundle exec jekyll build or bundle exec jekyll serve in the docs directory. (Update: Ignoring, as advised)

  2. Instead of navigating, clicking the “Reference” button on http://127.0.0.1:4000 duplicates the heading (GIF below). No errors appear in Terminal or the Safari Developer Tools Console. (Update: Fixed in b7fc3d4)

catalyst-heading-duplicated

@smockle smockle requested a review from a team as a code owner December 5, 2020 02:53
@github-pages github-pages Bot temporarily deployed to github-pages December 8, 2020 14:18 Inactive
Copy link
Copy Markdown
Contributor

@koddsson koddsson left a comment

Choose a reason for hiding this comment

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

This seems good to me 👍🏻

  1. The warning GitHub Metadata: No GitHub API authentication could be found. Some fields may be missing or have incorrect data. is logged when running bundle exec jekyll build or bundle exec jekyll serve in the docs directory.

I think this is fine if the page is otherwise working.

2. Instead of navigating, clicking the “Reference” button on http://127.0.0.1:4000 duplicates the heading (GIF below). No errors appear in Terminal or the Safari Developer Tools Console.

I can't reproduce locally 🤷🏻

Copy link
Copy Markdown
Contributor

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

👍

@smockle
Copy link
Copy Markdown
Contributor Author

smockle commented Dec 8, 2020

  1. Instead of navigating, clicking the “Reference” button on http://127.0.0.1:4000 duplicates the heading (GIF below). No errors appear in Terminal or the Safari Developer Tools Console.

I can't reproduce locally 🤷🏻

I can consistently repro it in Safari when the viewport is wider than 1280px. It seems to be related to the <br class="hide-sm"> in index.html—when I remove that break, things work as expected. Still debugging to figure out what’s going on, but I don’t think it’s related/caused by this PR’s update to Jekyll 4.x.

I suspect it’s something like Flexbug #11:

Safari uses min/max width/height declarations for actually rendering the size of flex items, but it ignores those values when calculating how many items should be on a single line of a multi-line flex container. Instead, it simply uses the item's flex-basis value, or its width if the flex basis is set to auto.

@github-pages github-pages Bot temporarily deployed to github-pages December 8, 2020 20:55 Inactive
@smockle
Copy link
Copy Markdown
Contributor Author

smockle commented Dec 8, 2020

  1. Instead of navigating, clicking the “Reference” button on http://127.0.0.1:4000 duplicates the heading (GIF below). No errors appear in Terminal or the Safari Developer Tools Console.

I can't reproduce locally 🤷🏻

I can consistently repro it in Safari when the viewport is wider than 1280px. It seems to be related to the <br class="hide-sm"> in index.html—when I remove that break, things work as expected. Still debugging to figure out what’s going on, but I don’t think it’s related/caused by this PR’s update to Jekyll 4.x.

I suspect it’s something like Flexbug #11:

Safari uses min/max width/height declarations for actually rendering the size of flex items, but it ignores those values when calculating how many items should be on a single line of a multi-line flex container. Instead, it simply uses the item's flex-basis value, or its width if the flex basis is set to auto.

Update: Fixed in b7fc3d4

@smockle smockle merged commit 5ca6556 into main Dec 8, 2020
@smockle smockle deleted the jekyll-v4 branch December 8, 2020 20:56
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