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.

Sign up for GitHub

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

Move featured chapter quotes from Jinja2 template to JSON file #1103

Closed
wants to merge 23 commits into from
Closed

Move featured chapter quotes from Jinja2 template to JSON file #1103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a7ddb6a
Change localizedChapter names to look up by slug
tunetheweb Jul 25, 2020
362a61f
Add Featured Chapters
tunetheweb Jul 25, 2020
31e1262
Move Features Chapters to JSON
tunetheweb Jul 25, 2020
a0d9f3c
Linter fixes
tunetheweb Jul 25, 2020
5893069
Linter fixes attempt 2
tunetheweb Jul 25, 2020
577e3c5
Linter fixes 3
tunetheweb Jul 25, 2020
d7bab37
Remove debug
tunetheweb Jul 25, 2020
25bc0b9
Add translations
tunetheweb Jul 25, 2020
42eac04
Fix typo
tunetheweb Jul 25, 2020
9964323
Merge branch 'main' into featured_quotes_v2
tunetheweb Jul 26, 2020
0260928
Merge branch 'main' into featured_quotes_v2
tunetheweb Jul 26, 2020
b6f95e2
Only regen quote on reload
tunetheweb Jul 26, 2020
dc32569
Merge branch 'main' into featured_quotes_v2
tunetheweb Jul 28, 2020
ab7df07
Add Simplified Chinese
tunetheweb Jul 28, 2020
e0fc312
Remove old featured_chapters
tunetheweb Jul 28, 2020
df6b905
Fix link
tunetheweb Jul 28, 2020
a493cc9
Merge branch 'main' into featured_quotes_v2
tunetheweb Jul 30, 2020
0ce1762
Code cleanup
tunetheweb Jul 30, 2020
cb652bc
Review feedback
tunetheweb Jul 31, 2020
2bebccc
Fix two consts
tunetheweb Jul 31, 2020
9af1af2
Fix whitespace
tunetheweb Jul 31, 2020
8784dbd
Merge branch 'main' into featured_quotes_v2
tunetheweb Aug 6, 2020
d1f1eb6
Bug fixes
tunetheweb Aug 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import os.path
import re
import sys
import json

# Set WOFF and WOFF2 caching to return 1 year as they should never change
# Note this requires similar set up in app.yaml for Google App Engine
Expand Down Expand Up @@ -190,6 +191,15 @@ def accentless_sort(value):
return sorted(value, key=lambda i: strip_accents(i[1]).lower())


def get_featured_quotes(lang, year):
featured_quotes = {}
featured_quote_filename = 'templates/%s/%s/featured_chapters.json' % (lang, year)
if os.path.isfile(featured_quote_filename):
with open(featured_quote_filename, 'r') as featured_quotes_file:
featured_quotes = json.load(featured_quotes_file)
Copy link
Member

Choose a reason for hiding this comment

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

Should we cache this in memory?

return featured_quotes


# Make these functions available in templates.
app.jinja_env.globals['get_view_args'] = get_view_args
app.jinja_env.globals['chapter_lang_exists'] = chapter_lang_exists
Expand All @@ -205,7 +215,8 @@ def accentless_sort(value):
@validate
def home(lang, year):
config = get_config(year)
return render_template('%s/%s/index.html' % (lang, year), config=config)
featured_quotes = get_featured_quotes(lang, year)
return render_template('%s/%s/index.html' % (lang, year), config=config, featured_quotes=featured_quotes)


@app.route('/<lang>/')
Expand Down
95 changes: 94 additions & 1 deletion src/templates/base/2019/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,100 @@ <h2>{{ self.intro_sub_title() }}</h2>
<img src="/static/images/home-hero.png" alt="" width="562" height="820">
</div>
</section>
{% include "%s/2019/featured_chapters.html" % lang %}
{% if featured_quotes %}
{# Get a random quote server side for those without JS #}
{%- set random_chapter = featured_quotes.keys()|list()|random %}
{%- set featured_chapter_quotes = featured_quotes.get(random_chapter) %}
{%- if not featured_chapter_quotes.get('stat_1') %}{% set stat1_hidden = "hidden" %}{% endif %}
{%- if not featured_chapter_quotes.get('stat_2') %}{% set stat2_hidden = "hidden" %}{% endif %}
{%- if not featured_chapter_quotes.get('stat_3') %}{% set stat3_hidden = "hidden" %}{% endif %}
{% if featured_chapter_quotes %}
<script nonce="{{ csp_nonce() }}">
// Get a random quote using JS for those reloading cached CDN page.
// We do as much as possible before HTML so we can replace the server
// generated quote quickly without a flash of the server side quote
Comment on lines +55 to +57
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sold on this. The contributors page was one thing because I can see the value of giving each contributor equal prominence. But a chapter can be featured for a few hours at a time and I don't think there's any expectation from readers or authors that this should randomly update on refresh.

The overhead in both the HTML payload and the JS code just don't seem worth it for a feature that would be used infrequently and adds little to the UX.

If you're still convinced that this is the right change, could we continue this discussion in another issue and decouple it from the JSON change in this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah you’re probably right. Having a chapter “featured” for a few hours is more expected. Was so preoccupied with whether or not I could that they didn't stop to think if I should 😁

Given that, should we even move to JSON or just stick with the Jinja2 template instead? We could still dynamically generate that with an EJS template like we do for the ebook. It would be an extra EJS template but as long as we use the .ejs.html naming convention we use for ebook that should be fine. Or do you think we will ever use these quotes elsewhere where having them in JSON format would be helpful?

var chapter_names = {{ localizedChapterTitles|tojson }}
var featured_chapters = {{ featured_quotes|tojson }}

var randomChapter="";

// Only run this if the page is reloaded for performance reasons.
// Support both newer PerformanceNavigationTiming API for newer browsers
// and older Navigation Timing API for older browsers
if ((performance.getEntriesByType("navigation")[0] && performance.getEntriesByType("navigation")[0].type === "reload")
||
(performance.navigation && performance.navigation.type === performance.navigation.TYPE_RELOAD)) {
var keys = Object.keys(featured_chapters)
var randIndex = Math.floor(Math.random() * keys.length)
randomChapter = keys[randIndex];
}
</script>
<section id="featured-chapter" class="featured-chapter">
<div class="featured-chapter-content">
<h2 class="title title-center">{{ self.featured_chapter() }}</h2>
<h3 class="featured-chapter-name">{{ localizedChapterTitles[random_chapter] }}</h3>
<blockquote id="featured-quote">
{{ featured_chapter_quotes.get('quote')|safe }}
</blockquote>
<div class="featured-chapter-content-data">
<div class="featured-chapter-content-data-item">
<div id="featured-stat-1" {{ stat1_hidden }}>{{ featured_chapter_quotes.get('stat_1')|safe }}</div>
<div id="featured-stat-label-1" {{ stat1_hidden }}>{{ featured_chapter_quotes.get('stat_label_1')|safe }}</div>
</div>
<div class="featured-chapter-content-data-item">
<div id="featured-stat-2" {{ stat2_hidden }}>{{ featured_chapter_quotes.get('stat_2')|safe }}</div>
<div id="featured-stat-label-2" {{ stat2_hidden }}>{{ featured_chapter_quotes.get('stat_label_2')|safe }}</div>
</div>
<div class="featured-chapter-content-data-item">
<div id="featured-stat-3" {{ stat3_hidden }}>{{ featured_chapter_quotes.get('stat_3')|safe }}</div>
<div id="featured-stat-label-3" {{ stat3_hidden }}>{{ featured_chapter_quotes.get('stat_label_3')|safe }}</div>
</div>
</div>
<a id="featured-link" href="{{ url_for('chapter', year=year, chapter=random_chapter, lang=lang) }}" class="btn">
{{ readChapter(featured_chapter_name) }}
</a>
</div>
</section>
{% endif %}
<script nonce="{{ csp_nonce() }}">

// Replace the quote with the randomly generated one
if (randomChapter) {
var featured_quote = document.querySelector('#featured-quote');
if (featured_quote) {
featured_quote.innerHTML = featured_chapters[randomChapter].quote;
}

var featured_titles = document.querySelectorAll('.featured-chapter-name');
for (let i = 0; i < featured_titles.length; i++) {
featured_titles[i].innerHTML = chapter_names[randomChapter];
}

for (let i=1; i <= 3; i++) {
var stat = featured_chapters[randomChapter]['stat_' + i];
var stat_label = featured_chapters[randomChapter]['stat_label_' + i];
var stat_node = document.querySelector('#featured-stat-' + i);
var stat_label_node = document.querySelector('#featured-stat-label-' + i);
if (stat_node && stat_label_node) {
if (stat && stat_label) {
stat_node.innerHTML = stat;
stat_label_node.innerHTML = stat_label;
stat_node.hidden = false;
stat_label_node.hidden = false;
} else {
stat_node.hidden = true;
stat_label_node.hidden = true;
}
}
}

var featured_link = document.querySelector('#featured-link');
if (featured_link) {
featured_link.href = document.location.pathname + randomChapter;
}
}
</script>
{% endif %}
<section id="contributors" class="contributors-container alt-bg">
<div class="contributors">
<h2 class="title title-alt">{{ self.contributors_title() }}</h2>
Expand Down
Loading