Skip to content

Commit

Permalink
Learning Track navigation banner (github#17440)
Browse files Browse the repository at this point in the history
* add middleware to handle `learn` query param

* add exception to query-less cache key

* add querystring to learning track guides
  • Loading branch information
vanessayuenn authored Jan 25, 2021
1 parent 97dfb49 commit 9bc90cd
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 21 deletions.
3 changes: 3 additions & 0 deletions data/ui.yml
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,6 @@ product_sublanding:
tutorial: Tutorial
how_to: How-to guide
reference: Reference
learning_track_nav:
prevGuide: Previous Guide
nextGuide: Next Guide
5 changes: 4 additions & 1 deletion includes/article.html
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ <h2 id="in-this-article" class="f5 mb-2"><a class="link-gray-dark" href="#in-thi
</div>
</div>

<div class="d-block border-top border-gray-light mt-4 markdown-body">
<div class="d-block mt-4 markdown-body">
{% if currentLearningTrack and currentLearningTrack.trackName %}
{% include learning-track-nav %}
{% endif %}
{% include helpfulness %}
{% unless page.hidden %}{% include contribution %}{% endunless %}
</div>
Expand Down
18 changes: 18 additions & 0 deletions includes/learning-track-nav.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<div class="py-3 px-4 rounded bg-white border-gradient--purple-pink d-flex flex-justify-between learning-track-nav">
{% assign track = currentLearningTrack %}

<span class="d-flex flex-column">
{% if track.prevGuide %}
<span class="f6 text-gray">{% data ui.learning_track_nav.prevGuide %}</span>
<a href="{{track.prevGuide.href}}?learn={{track.trackName}}" class="text-bold text-gray">{{track.prevGuide.title}}</a>
{% endif %}
</span>

<span class="d-flex flex-column flex-items-end">
{% if track.nextGuide %}
<span class="f6 text-gray">{% data ui.learning_track_nav.nextGuide %}</span>
<a href="{{track.nextGuide.href}}?learn={{track.trackName}}" class="text-bold text-gray text-right">{{track.nextGuide.title}}</a>
{% endif %}
</span>

</div>
10 changes: 5 additions & 5 deletions layouts/product-sublanding.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ <h1 class="my-3 font-mktg">{{ page.shortTitle }}</h1>
<div class="circle bg-white text-blue border border-white d-inline-flex">{% octicon "star-fill" height="24" class="v-align-middle m-2"%}</div>
<h3 class="font-mktg h2-mktg my-4">{{ featuredTrack.title }}</h3>
<div class="lead-mktg text-white f5 my-4">{{ featuredTrack.description }}</div>
<a class="d-inline-block border border-white text-white px-4 py-2 f5 no-underline text-bold" role="button" href="{{ featuredTrack.guides[0].href }}">
<a class="d-inline-block border border-white text-white px-4 py-2 f5 no-underline text-bold" role="button" href="{{ featuredTrack.guides[0].href }}?learn={{ featuredTrack.trackName }}">
<span class="mr-2">{% octicon "arrow-right" height="20" %}</span>
{% data ui.product_sublanding.start_path %}
</a>
</div>
</li>
{% for guide in featuredTrack.guides %}
<li class="px-2 d-flex flex-shrink-0">
<a href="{{ guide.href }}" class="d-inline-block Box p-5 bg-white border-gray no-underline">
<a href="{{ guide.href }}?learn={{ featuredTrack.trackName }}" class="d-inline-block Box p-5 bg-white border-gray no-underline">
<div class="d-flex flex-justify-between flex-items-center">
<div class="circle bg-white text-blue border-gradient--purple-pink d-inline-flex">
<span class="m-2 f2 lh-condensed-ultra text-center text-bold text-gradient--blue-purple" style="width: 24px; height: 24px;">{{ forloop.index }}</span>
Expand All @@ -61,7 +61,7 @@ <h2 class="mb-3 font-mktg">{% data ui.product_sublanding.learning_paths %}</h2>
<!-- Learning tracks -->
<div class="d-flex flex-wrap flex-items-start my-5">
{% for track in page.learningTracks offset:1 %}
<div class="my-3 px-0 px-4 col-12 col-md-6">
<div class="my-3 px-0 px-4 col-12 col-md-6 learning-track">
<div class="Box js-show-more-container">
<div class="Box-header bg-gradient--purple-pink py-4 d-flex flex-auto flex-items-start flex-wrap">
<div class="d-flex flex-auto flex-items-start col-8 col-md-12 col-xl-8">
Expand All @@ -75,14 +75,14 @@ <h4 class="mb-3 text-white font-mktg h3-mktg ">
<p class="text-white">{{ track.description }}</p>
</div>
</div>
<a class="d-inline-block border border-white text-white px-3 py-2 f5 no-underline text-bold no-wrap" role="button" href="{{ track.guides[0].href }}">
<a class="d-inline-block border border-white text-white px-3 py-2 f5 no-underline text-bold no-wrap" role="button" href="{{ track.guides[0].href }}?learn={{ track.trackName }}">
{% data ui.product_sublanding.start %}
<span class="ml-2">{% octicon "arrow-right" height="20" %}</span>
</a>
</div>
<div>
{% for guide in track.guides %}
<a class="Box-row d-flex flex-items-center text-gray-dark no-underline js-show-more-item {% if forloop.index > 4 %}d-none{% endif %}" href="{{ guide.href }}">
<a class="Box-row d-flex flex-items-center text-gray-dark no-underline js-show-more-item {% if forloop.index > 4 %}d-none{% endif %}" href="{{ guide.href }}?learn={{ track.trackName }}">
<div class="circle bg-gray d-inline-flex mr-4">
<span class="m-2 f3 lh-condensed-ultra text-center text-bold text-gradient--purple-pink" style="min-width: 20px; height: 20px;">{{ forloop.index }}</span>
</div>
Expand Down
45 changes: 31 additions & 14 deletions lib/get-link-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,45 @@ const removeFPTFromPath = require('./remove-fpt-from-path')

// rawLinks is an array of paths: [ '/foo' ]
// we need to convert it to an array of localized objects: [ { href: '/en/foo', title: 'Foo', intro: 'Description here' } ]
module.exports = async (rawLinks, context) => {
module.exports = async (rawLinks, context, option = { title: true, intro: true }) => {
if (!rawLinks) return

if (typeof rawLinks === 'string') {
return await processLink(rawLinks, context, option)
}

const links = []

for (const link of rawLinks) {
const linkPath = link.href || link
const version = context.currentVersion === 'homepage' ? nonEnterpriseDefaultVersion : context.currentVersion
const href = removeFPTFromPath(path.join('/', context.currentLanguage, version, linkPath))
const linkObj = await processLink(link, context, option)
if (!linkObj) {
continue
} else {
links.push(linkObj)
}
}

return links
}

const processLink = async (link, context, option) => {
const linkPath = link.href || link
const version = context.currentVersion === 'homepage' ? nonEnterpriseDefaultVersion : context.currentVersion
const href = removeFPTFromPath(path.join('/', context.currentLanguage, version, linkPath))

const linkedPage = findPage(href, context.pages, context.redirects)
if (!linkedPage) return null

const linkedPage = findPage(href, context.pages, context.redirects)
if (!linkedPage) continue
const opts = { textOnly: true, encodeEntities: true }

const opts = { textOnly: true, encodeEntities: true }
const result = { href, page: linkedPage }

links.push({
href,
title: await linkedPage.renderTitle(context, opts),
intro: await linkedPage.renderProp('intro', context, opts),
page: linkedPage
})
if (option.title) {
result.title = await linkedPage.renderTitle(context, opts)
}

return links
if (option.intro) {
result.intro = await linkedPage.renderProp('intro', context, opts)
}
return result
}
1 change: 1 addition & 0 deletions lib/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ class Page {
const track = context.site.data['learning-tracks'][context.currentProduct][trackName]
if (!track) continue
learningTracks.push({
trackName,
title: await renderContent(track.title, context, { textOnly: true, encodeEntities: true }),
description: await renderContent(track.description, context, { textOnly: true, encodeEntities: true }),
guides: await getLinkData(track.guides, context)
Expand Down
1 change: 1 addition & 0 deletions middleware/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ module.exports = function (app) {
app.use(require('./enterprise-server-releases'))
app.use(require('./dev-toc'))
app.use(require('./featured-links'))
app.use(require('./learning-track'))

// *** Rendering, must go last ***
app.get('/*', asyncMiddleware(require('./render-page')))
Expand Down
40 changes: 40 additions & 0 deletions middleware/learning-track.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const { getPathWithoutLanguage, getPathWithoutVersion } = require('../lib/path-utils')
const getLinkData = require('../lib/get-link-data')

module.exports = async (req, res, next) => {
const noTrack = () => {
req.context.currentLearningTrack = {}
return next()
}

if (!req.context.page) return next()

const trackName = req.query.learn
if (!trackName) return noTrack()

const track = req.context.site.data['learning-tracks'][req.context.currentProduct][trackName]
if (!track) return noTrack()

const currentLearningTrack = { trackName }

const guidePath = getPathWithoutLanguage(getPathWithoutVersion(req.path))
const guideIndex = track.guides.findIndex((path) => path === guidePath)

if (guideIndex < 0) return noTrack()

if (guideIndex > 0) {
const prevGuidePath = track.guides[guideIndex - 1]
const { href, title } = await getLinkData(prevGuidePath, req.context, { title: true, intro: false })
currentLearningTrack.prevGuide = { href, title }
}

if (guideIndex < track.guides.length - 1) {
const nextGuidePath = track.guides[guideIndex + 1]
const { href, title } = await getLinkData(nextGuidePath, req.context, { title: true, intro: false })
currentLearningTrack.nextGuide = { href, title }
}

req.context.currentLearningTrack = currentLearningTrack

return next()
}
12 changes: 11 additions & 1 deletion middleware/render-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,21 @@ const pageCache = new RedisAccessor({
allowSetFailures: true
})

// a list of query params that *do* alter the rendered page, and therefore should be cached separately
const cacheableQueries = ['learn']

module.exports = async function renderPage (req, res, next) {
const page = req.context.page

// Remove any query string (?...) and/or fragment identifier (#...)
const originalUrl = new URL(req.originalUrl, 'https://docs.github.com').pathname
const { pathname, searchParams } = new URL(req.originalUrl, 'https://docs.github.com')

for (const queryKey in req.query) {
if (!cacheableQueries.includes(queryKey)) {
searchParams.delete(queryKey)
}
}
const originalUrl = pathname + ([...searchParams].length > 0 ? `?${searchParams}` : '')

// Serve from the cache if possible (skip during tests)
const isCacheable = !process.env.CI && process.env.NODE_ENV !== 'test' && req.method === 'GET'
Expand Down
62 changes: 62 additions & 0 deletions tests/rendering/learning-tracks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
const { getDOM } = require('../helpers/supertest')

jest.setTimeout(3 * 60 * 1000)

describe('learning tracks', () => {
test('render first track as feature track', async () => {
const $ = await getDOM('/en/actions/guides')
expect($('.feature-track')).toHaveLength(1)
const href = $('.feature-track li a').first().attr('href')
const found = href.match(/.*\?learn=(.*)/i)
expect(found).not.toBeNull()
const trackName = found[1]

// check all the links contain track name
$('.feature-track li a').each((i, elem) => {
expect($(elem).attr('href')).toEqual(expect.stringContaining(`?learn=${trackName}`))
})
})

test('render other tracks', async () => {
const $ = await getDOM('/en/actions/guides')
expect($('.learning-track').length).toBeGreaterThanOrEqual(4)
$('.learning-track').each((i, trackElem) => {
const href = $(trackElem).find('.Box-header a').first().attr('href')
const found = href.match(/.*\?learn=(.*)/i)
expect(found).not.toBeNull()
const trackName = found[1]

// check all the links contain track name
$(trackElem).find('a.Box-row').each((i, elem) => {
expect($(elem).attr('href')).toEqual(expect.stringContaining(`?learn=${trackName}`))
})
})
})
})

describe('navigation banner', () => {
test('render navigation banner when url includes correct learning track name', async () => {
const $ = await getDOM('/en/actions/guides/setting-up-continuous-integration-using-workflow-templates?learn=continuous_integration')
expect($('.learning-track-nav')).toHaveLength(1)
const $navLinks = $('.learning-track-nav a')
expect($navLinks).toHaveLength(2)
$navLinks.each((i, elem) => {
expect($(elem).attr('href')).toEqual(expect.stringContaining('?learn=continuous_integration'))
})
})

test('does not include banner when url does not include `learn` param', async () => {
const $ = await getDOM('/en/actions/guides/setting-up-continuous-integration-using-workflow-templates')
expect($('.learning-track-nav')).toHaveLength(0)
})

test('does not include banner when url has incorrect `learn` param', async () => {
const $ = await getDOM('/en/actions/guides/setting-up-continuous-integration-using-workflow-templates?learn=not_real')
expect($('.learning-track-nav')).toHaveLength(0)
})

test('does not include banner when url is not part of the learning track', async () => {
const $ = await getDOM('/en/actions/learn-github-actions/introduction-to-github-actions?learn=continuous_integration')
expect($('.learning-track-nav')).toHaveLength(0)
})
})

0 comments on commit 9bc90cd

Please sign in to comment.