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

[WIP] Quicklink site #141

Merged
merged 6 commits into from Dec 1, 2019
Merged

[WIP] Quicklink site #141

merged 6 commits into from Dec 1, 2019

Conversation

addyosmani
Copy link
Collaborator

This adds our Eleventy-based Quicklink site to master ⭐️

We've been iterating on this across email, Glitch and in-person discussions so it's had a little time to evolve. Our initial intention is to land this base and unlock contributors who have been working on related guides to file their PRs on top.

There are aspects of the site I wouldn't consider final just yet:

  • List of highlighted sites using Quicklink. We have a few more recent ones we should include.
  • We could do a better job linking out to demos. We can take a look at this soon.
  • Tagline for the site. This is based on the average conversion wins we've seen from sites we have spoken to, but the sentence structure feels similar to instant.page. I'd like to see if we can improve.

Huge kudos to Anton, Demian, Jonathan, Milica and many others who have helped shape this in conversations across a few places.
@addyosmani
Copy link
Collaborator Author

@demianrenzulli added you as an informal reviewer... :)

One thing we may want to decide before landing the MVP is deployment 🚀Given the output here is a relatively static site, we could go with one of 3 options:

  • Deploy to gh-pages
  • Deploy to Firebase (*.web.app)
  • Deploy to one of the above or standard GCP hosting and a custom domain (which could come later)

I don't really have strong preferences. Firebase/GCP give us a little more control should we need to go more custom with our infrastructure (e.g adding Brotli compression) but gh-pages are free and might serve our needs fine for a few quarters given it's a simple site.

Any preferences?

* master:
  [infra] Fix tests (#142)
Copy link
Contributor

@demianrenzulli demianrenzulli left a comment

Choose a reason for hiding this comment

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

The site looks great. Thanks for instructions notes to the README on how to build and run. Tested it and works fine 👍
Also, LH score is 93! With the only recommendation of "Preconnect to required origins" for https://api.github.com/, which doesn't seem to be really necessary.

Leaving a couple of observations before approving.

  • Usage of partials: Do you think that contributors should follow the pattern of creating a partial for every component in the page applied in index.njk, or you have applied this style here because you think this page has mostly components that can be reused more frequently somewhere else?

  • HTML vs markdown: While index.njk consists mostly of HTML code, the rest of the sections have been written almost completely in markdown language ({% markdownConvert %}). I guess this is because markdown worked just fine for sections like /api, /approach and /demo, which have been extracted from github README.md, but it was fairly limited for the home page, which uses it only in certain areas.

  • Nav bar: The current nav bar is handy and looks nice, but I'm wondering how it will scale as new sections are added to the site. Perhaps leaving the current main categories as they are, and adding a side menu in the left inside each section might work? See in this jquery site example: https://api.jquery.com/.

  • Analytics: Haven't seen any analytics script. Perhaps not necessary... Wondering there's any kind of information you might find interesting to track? Usefulness of docs, for example? Not really critical for this initial release.

@addyosmani
Copy link
Collaborator Author

The site looks great. Thanks for instructions notes to the README on how to build and run. Tested it and works fine 👍

I'm glad you like the current state of the site. Team effort! We also now have a domain :) https://getquick.link 😄

Also, LH score is 93! With the only recommendation of "Preconnect to required origins" for https://api.github.com/, which doesn't seem to be really necessary.

Yeah, I don't think we're relying on that end-point for anything super critical path. I'll take a look at whether it's worth pre-connecting to the origin just in case.

HTML vs markdown: While index.njk consists mostly of HTML code, the rest of the sections have been written almost completely in markdown language ({% markdownConvert %}). I guess this is because markdown worked just fine for sections like /api, /approach and /demo, which have been extracted from github README.md, but it was fairly limited for the home page, which uses it only in certain areas.

Usage of partials: Do you think that contributors should follow the pattern of creating a partial for every component in the page applied in index.njk, or you have applied this style here because you think this page has mostly components that can be reused more frequently somewhere else?

I think it's fine for contributors to follow the pattern of a new partial for each component (...where it makes sense) to keep things as isolated and modular as possible. I'm picturing as the site grows that we will make use of a lot more re-usage than we do at present.

HTML vs markdown: While index.njk consists mostly of HTML code, the rest of the sections have been written almost completely in markdown language ({% markdownConvert %}). I guess this is because markdown worked just fine for sections like /api, /approach and /demo, which have been extracted from github README.md, but it was fairly limited for the home page, which uses it only in certain areas.

Yes, this is correct. At some point I would love for us to be able to easily just pull in markdown content from the repo for the site, without there being duplication between it and the README. For now, I would say the index.njk containing mostly HTML code while we try to shift towards just relying on markdown elsewhere should be okay.

Nav bar: The current nav bar is handy and looks nice, but I'm wondering how it will scale as new sections are added to the site. Perhaps leaving the current main categories as they are, and adding a side menu in the left inside each section might work? See in this jquery site example: https://api.jquery.com/.

So, funny story... :) We did originally have a proper sidebar but decided to move away from it once we realized we didn't have enough menu items to popular it on mobile. I think it wouldn't be a lot of work to bring this back in when it is needed. So, +1 to the idea.

Analytics: Haven't seen any analytics script. Perhaps not necessary... Wondering there's any kind of information you might find interesting to track? Usefulness of docs, for example? Not really critical for this initial release.

This is a good catch. I think we should at least add in GA. Having stats on how often folks visit the docs site vs just the repo would help us determine how much effort to put into the site when time is tight. I'll add a commit incorporating GA :)

@demianrenzulli
Copy link
Contributor

Great! Will await your commit for Analytics before approving. 👍

function gtag() { dataLayer.push(arguments); }
gtag('js', new Date());

gtag('config', 'UA-153597376-1');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here's GA, @demianrenzulli :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect!

@addyosmani
Copy link
Collaborator Author

@demianrenzulli PR updated. I've also refreshed the docs and site to use Quicklink 2.0.0 alpha as I noticed it was still based on 1.0.1.

Copy link
Contributor

@demianrenzulli demianrenzulli left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, approving it now. Please, feel free to merge.

@demianrenzulli demianrenzulli merged commit 468b4ec into master Dec 1, 2019
@demianrenzulli
Copy link
Contributor

@addyosmani just merged it, hope it's fine!

@addyosmani
Copy link
Collaborator Author

Thanks!

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.

None yet

2 participants