-
-
Notifications
You must be signed in to change notification settings - Fork 182
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
Add randomized featured chapters #614
Conversation
e7a112e
to
c08af7b
Compare
Sorry, I wanted to PR a multi-files suggestion, and ended up pushing on your branch. My bad. I've PR'ed on #615, and this should fix the FR version. I don't think repetition is an issue, as we already have to translate the template. A routine could automatically extract data from the chapters, but we would still have to do the curation of the key figures, so I don't know if it would save time. I don't have an opinion on how long the cache should last. |
That’s cool - thanks for that.
It could also extract the metrics as they are also in the chapters, but the metric labels would probably have to be hidden spans in the chapter itself as they are not direct quotes. All in all I’m feeling it’s more effort than its worth which is why I just embedded straight into index.html rather than some complex extraction from the chapters and insertion on the fly.
Cool. Was more tagging you as a reviewer for the French translation part but always good to get opinions of people have them! |
9a25782
to
c08af7b
Compare
* Using the safe filter to include content that may contain HTML * Small typo * Small fixes: - Add non-break space before ":" - Insert "(3P)" into the paragraph for Third Parties to identify the abbreviation as it is used after, in the key numbers
Any thoughts on this @rviscomi and @mikegeyser ? |
Any thoughts on removing the featured chapter section entirely? Not sure it's worth the complexity of maintaining a fresh set of quotes/stats. |
I like it actually. Entices you into an example chapter I think. And the point of this change (once we add quotes for the other 18 chapters) is that there isn't any maintenance - except if we correct any of the wording or stats which I would hope would be rare now. You just get a random quote each time you refresh the page. We could limit it to a few quotes if we don't want one for each chapter to reduce the effort and maintenance. Also this feature might encourage reading of the lesser-known chapters. Since 1st Nov, Google Analytics has Third-Parties chapter as sixth most read chapter, and it shows 576 of the 2,909 pages views came from this link so that's quite a chunk of that chapters views (other chapters have much less links from home page - performance has 24 out of 3932, and HTTP/2 has 47 out of 2,143 to look at our own chapters as examples). I suspect it would get more usage if it changed more frequently. Conter-argument is wording can get out of sync so yeah there might be some maintenance, and having it means there's more to translate (though this will mostly be handed by chapter translations anyway). |
Or possibly 716 clicks. Different things in different parts of Google Analytics (Behaviour Flow has 576, but Behaviour report and Page Analytics extension both have 716). Either way it's a significant chunk of it's traffic - especially compared to other chapters which get little enough traffic directly from the home page (surprised they get any as no links - I guess people typing URL?). |
That's the part that frightens me. This PR covers two chapters and the Jinja variables are already starting to feel unwieldy. 10x as many variables will really get out of hand. Two possible alternatives:
The traffic to the home page today is extremely small relative to launch, so I don't expect much more visibility for other chapters. But I agree with the spirit behind this PR to showcase other chapters equally, so I'd at least like to find a low-effort way to do that. |
That's a fair point. I've made two changes to try to alleviate that concern:
Have a look at the latest changes and see what you think and it that alleviates your concerns somewhat.
Possibly, but still think it's the right thing to do, and will also help when launching other languages. Plus I'm also think about the future and when we launch 2020 edition. The site might change but anything like this that we solve now with the 2019 site and design, would potentially make it easier to have a more fully featured site, with items like this, for that launch (assuming we stick with the same technologies even if the site changes). |
Any more thoughts on this @rviscomi ? I'm still a fan of the featured chapter section and think it's a nice feature of the home page and stats seem to indicate it is used. Moving this into its own Jinja template will also hopefully address some of the concerns about the index.html template getting unwieldy. I'm hoping that the discussions in #634 and #638 that having a dynamic home page (as this would require) is not a performance problem given we have Google Cloud in front of this. Anyway, I think we should make a decision and either continue to work on this and add the remaining chapters, or close it if we want to remove the featured chapter section. |
Thanks for the ping. The changes do help alleviate my concerns but I'm still hesitant to go down this route. It feels redundant to maintain snippets of each chapter in this template. I'd like to see the There are two other considerations: rendering the snippets and selecting a chapter to feature. I'm open to ideas but it may make sense to generate the template instead of constructing it manually. Selecting a featured chapter could be random or we could program it to cycle through chapters at regular intervals (daily, weekly). WDYT? |
Cheers for getting back to me. I did talk about getting the data from the md files above (see #614 (comment)) but decided it was more complexity than it was worth, but maybe with the refactoring I've done since that's not longer the case. Will have a look. I still think it's fine to generate it at random, from the template of all featured chapters if it was generated by Doesn't seem a way to change a PR to a Draft PR and would like to keep the history so will keep this open for now and add to it when I get a chance. |
@rviscomi I spent some time looking to auto generate the content from the chapters themselves but after a number of different attempts, I really think that would:
So I've abandoned working on that. Really don't think it's the right way to go. I've added quotes and stats for all the other chapters into this PR so you can see the real size and complexity of the page. At 100 lines of config for all chapters I think it's fine, especially now it is separated to a separate template file so as not to clutter the index.html template. It also still seems quick to generate (I didn't see any significant slow down when generating locally) and the Google CDN will cache it for 3 hours anyway during which time visitors will be getting the pre-generated cached version. So it's still my opinion that this is something we should do and I think it's in a good place to merge now. I've also included the French version, but restricted it to just pick from the one translated snippet for now. One of the French translators can pick this up to close out if this is accepted. |
@bazzadp sorry for the delay! Catching up today. I don't think I did a good job of explaining what I had in mind. It sounds like you spent a lot of time exploring how to generate the featured snippets from the md content itself, but what I pictured was for the authors/editors to write a quote or some stats to highlight in the yaml metadata at the top of the md file. Each chapter can fall back to the existing description field, otherwise we can write a custom featured snippet with stats. The challenge is to generate a py or jinja file with all of the snippets for easy consumption by the home page template. I'm pushing for this approach because it keeps all of the chapter metadata in one place next to the content itself and translators only need to worry about that one file. Given that clarification, is this something you'd reconsider? |
It is something I'd consider, but given that would just automate creation of the That would at least avoid me having to keep up with my other Plus it would make it a massive PR as would need to change each .md file so, again think we're better to merge this as is for now to make that PR smaller. |
SGTM. Can you summarize this discussion in #373?
For the record, if we default to the existing |
Ah the description is not a great quote. It's for totally different purposes (SEO and structured data) and is also limited in length because of that. Compare this: Description:
Featured Chapter Quote:
|
Makes progress on #373
So I thought it was time the Third Party chapter stopped stealing all the limelight on the front page 😀so this PR adds the ability for other chapters to become the featured chapter.
However, there's some questions/design choices I made so would appreciate some feedback:
index.html
template, rather than pulling them from the chapter markdown/templates. Again this is repetitive and also risks them getting out of sync. On the other hand it allows tweaks to the actual text (the HTTP/2 quote for example is not a direct quote and tweaks the last sentence from the one that appears in the actual chapter). It's also probably more performant, and less complex, than pulling these in from a separatequotes.json
file generated bynpm run generate
or similar like I suggested in Randomize Featured Chapter and Quote #373 (comment).On the plus side it works (or at least appears to from my testing!), and there are good reasons for the choices I've made. And we can always iterate if want to improve once things like #561 have landed.
Thoughts?