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

Update website styling and add Google site verification #408

Merged
merged 13 commits into from
Dec 30, 2022
4 changes: 4 additions & 0 deletions website/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ If you are not familiar with Jekyll, be sure to check out its
[documentation][jekyll-docs]. It is recommended to follow the provided
step-by-step tutorial.

We use the [Just the Docs][just-the-docs] Jekyll theme, which also includes
several configuration options.

###### Switch Ruby versions

The required Ruby version is set in `.ruby-version`. To switch, you can use
Expand Down Expand Up @@ -56,5 +59,6 @@ Actions workflow any time a change is merged to `master`.
[jekyll]: https://jekyllrb.com
[jekyll-docs]: https://jekyllrb.com/docs
[jekyll-docs-installation]: https://jekyllrb.com/docs/installation
[just-the-docs]: https://just-the-docs.github.io/just-the-docs/
[localhost-port-4000]: http://127.0.0.1:4000
[rvm]: https://rvm.io
6 changes: 6 additions & 0 deletions website/_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ nav_external_links:
url: https://github.com/PicnicSupermarket/error-prone-support
hide_icon: false

callouts:
summary:
color: blue
note:
color: grey-dk
Comment on lines +31 to +35
Copy link
Member

Choose a reason for hiding this comment

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

Just checking: this will only be used in the next PR, right?

Copy link
Member Author

@Badbond Badbond Dec 29, 2022

Choose a reason for hiding this comment

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

Correct. All changes in this PR won't be effective on master yet but are (almost all -- cases where we didn't, I tested locally against the website branch) already present on the website branch and therefore deployed. After @rickie's docgen modules, I'll open a PR to move the Ruby templating logic and a subsequent PR to change the GHA logic.


# SEO configuration.
# See https://jekyll.github.io/jekyll-seo-tag/usage.
social:
Expand Down
4 changes: 2 additions & 2 deletions website/_includes/footer_custom.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<img src="/assets/images/[email protected]" alt="Picnic Logo" id="logo" />
<img src="/assets/images/[email protected]" alt="Picnic Logo" id="logo"/>

<p align="center">
<p>
Copyright &copy; 2017-2022 Picnic Technologies BV
</p>
12 changes: 6 additions & 6 deletions website/_includes/head_custom.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
<link rel="icon" type="image/png" sizes="16x16" href="/assets/images/favicon-16x16.png">
<link rel="manifest" href="/assets/images/site.webmanifest">
<link rel="mask-icon" href="/assets/images/safari-pinned-tab.svg" color="#5bbad5">
<link rel="shortcut icon" href="/favicon.ico">
<meta name="msapplication-TileColor" content="#da532c">
<meta name="msapplication-config" content="/assets/images/browserconfig.xml">
<meta name="theme-color" content="#ffffff">

<!-- XXX: The theme does not natively support both light and dark mode. Drop
this section once https://github.com/just-the-docs/just-the-docs/issues/234 is
resolved. -->
<!-- Support light and dark mode, as it's not natively supported. See
https://github.com/just-the-docs/just-the-docs/issues/234. -->
<link rel="stylesheet" href="{{ '/assets/css/just-the-docs-eps-light.css' | relative_url }}"
media="(prefers-color-scheme: light)">
media="(prefers-color-scheme: light)">
Copy link
Member

Choose a reason for hiding this comment

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

These spaces seem a bit weird 👀. Is that because of a formatter?

Copy link
Member Author

@Badbond Badbond Dec 12, 2022

Choose a reason for hiding this comment

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

Seems to have changed during cherry-pick from @oxkitsune's branch to mine. This formatting happens to me as part of IntelliJ formatting (CTRL-ALT-L). Currently no CI formatting/linting is in place for these files, nor could I find any so quickly to run it as a one-off. I'm happy with reverting if you'd like. If we go with IntelliJ formatting for now, I'll remove the space at the last line before /> too (committed).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah let's do IntelliJ formatting for now :).

<link rel="stylesheet" href="{{ '/assets/css/just-the-docs-eps-dark.css' | relative_url }}"
media="(prefers-color-scheme: dark)">
media="(prefers-color-scheme: dark)">

<meta name="google-site-verification" content="2GBzy2ufS8Rfqffu8T6iqng6dbDw9EKuykMisUZU3IQ"/>
17 changes: 16 additions & 1 deletion website/_sass/color_schemes/_common.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
footer > img#logo {
// Add support for external anchor icons.
Copy link
Member

Choose a reason for hiding this comment

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

This "style" of writing sounds like an XXX to me. WDYT about changing it to: "Adds support for...". Not sure if that is the right way to go, but maybe it makes more sense 🤔.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think such comments are similar to in-code comments (Java example) which can carry the same mood. It is just a hard-to-explain feeling, but to me the style of 'Adds' is a bit awkward in this context and feels more fitting in JavaDoc-type of comments. 😅 But I'd be happy to change this, but then it should be applied in other places too.

Copy link
Member

Choose a reason for hiding this comment

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

We could meet in the middle and use /*-style comments here? No strong opinion though. (I agree that the grammar is correct as-is!)

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a difference in behavior between //-style and /* style commments, though. // do not get compiled to CSS and /* do end up in the compiled CSS. See SASS documentation. Not sure whether it is worth it while sending more bytes over the line.

Copy link
Member

Choose a reason for hiding this comment

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

TIL! Agree that we can then leave it as-is.

.external > svg {
width: 1rem;
vertical-align: text-bottom;
}

.label {
// Reduce spacing between labels and align with surrounding elements.
margin-left: 0 !important;
}

footer {
text-align: center;

img#logo {
width: 2rem;
margin: 0 auto;
display: block;
}
}
4 changes: 4 additions & 0 deletions website/_sass/color_schemes/_variables.scss
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
// Overrides for Just the Docs. See
// https://github.com/just-the-docs/just-the-docs/blob/main/_sass/support/_variables.scss.

// Grid system.
$nav-width: 400px;
12 changes: 12 additions & 0 deletions website/_sass/color_schemes/eps-dark.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
@import "./color_schemes/dark";
@import "_variables";
@import "_common";

// Swap `$blue-000` and `$blue-300`, mainly for callouts. This is done by
// default for red, but not for other colors.
$blue-000: #183385;
$blue-300: #2c84fa;

// Use light-theme greys in dark theme to have summary callouts stand out more.
$grey-dk-000: $grey-lt-000;
$grey-dk-100: $grey-lt-100;
$grey-dk-200: $grey-lt-200;
$grey-dk-250: $grey-lt-200;
$grey-dk-300: $grey-lt-300;
Comment on lines +12 to +16
Copy link
Member

Choose a reason for hiding this comment

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

Just checking: only 250 having an "off' mapping is intended?

Copy link
Member Author

@Badbond Badbond Dec 29, 2022

Choose a reason for hiding this comment

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

I think it is based on the colors defined by just-the-docs. I believe the idea was for the summary boxes to be light instead of dark in a dark theme, as otherwise it does not stand out as much. As you can see, the light grays only contain 4 entries instead of 5.

Copy link
Member

Choose a reason for hiding this comment

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

Wow 😄 (Maybe worth a comment; not sure.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. Added a comment 👍.