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

[Learn] Land the learn page #1258

Merged
merged 10 commits into from
Nov 9, 2023
Merged

[Learn] Land the learn page #1258

merged 10 commits into from
Nov 9, 2023

Conversation

AndrewJakubowicz
Copy link
Contributor

Fixes: #822

Context

"Land" the Learn catalog page. This PR does not remove the tutorials page, as that page is still valuable for the tutorials collection.

Changes

  • Change nav bar Tutorials -> Learn.
  • Change links to /tutorials -> /learn#filter=tutorial.

Test plan

Added a smoke test integration test, which renders the tutorial catalog.

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

🧑‍🏫 🚀

packages/lit-dev-server/src/redirects.ts Outdated Show resolved Hide resolved
packages/lit-dev-server/src/redirects.ts Outdated Show resolved Hide resolved
packages/lit-dev-server/src/redirects.ts Outdated Show resolved Hide resolved
@augustjk
Copy link
Member

augustjk commented Nov 7, 2023

Now that the article page is navigable, check links is failing on the hidden links to tags.
Currently tag links are part of the page but hidden with css:

We should just not render them at all until it's available by commenting out:

<div class="tags">
{% for tag in tags %}
{% if tag != 'articles-nav' %}
<a href="{{ site.baseurl }}/articles/tags/{{ tag }}/" aria-label="{{ tag }} tag">{{ tag }}</a>
{% endif %}
{% endfor %}
</div>

@augustjk
Copy link
Member

augustjk commented Nov 7, 2023

Oh no.. the link checker thinks # are only meant to be sections on the page, not data to be read with JS.

BROKEN REDIRECT /learn/#filter=tutorial
Could not find section matching hash #filter=tutorial.
Searched in file ../lit-dev-content/_site/learn/index.html

We may need to explicitly bypass this here

if (hash) {
// Another hack. Just do a regexp search for e.g. id="somesection" instead
// of DOM parsing. Should be good enough, especially given how regular our
// Markdown generated HTML is.
const idAttrRegExp = new RegExp(`\\sid=["']?${hash.slice(1)}["']?[\\s>]`);
if (data.match(idAttrRegExp) === null) {
return `Could not find section matching hash ${hash}.
Searched in file ${indexHtmlPath}`;
}

@AndrewJakubowicz
Copy link
Contributor Author

@augustjk Thank you for the redirect tip. I've added a small workaround so that we pass the tests.

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

Let's land it!! Hype!

@AndrewJakubowicz AndrewJakubowicz merged commit e148d6c into main Nov 9, 2023
15 checks passed
@AndrewJakubowicz AndrewJakubowicz deleted the land-learn-page branch November 9, 2023 21:39
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.

Feature tutorials catalog and YouTube content
3 participants