Accessibility and SEO improvements by jcabak · Pull Request #540 · google/docsy

jcabak · 2021-05-02T17:28:34Z

Background and foreground colours have a sufficient contrast ratio. (In “Docs” for date label “Last modified”)
Links have a discernible name
Added aria-label for links “read more” and “next” and previous” article in blog
Links to cross-origin destinations are safe (added rel="noopener")
Document have a meta description
Text remains visible during webfont load (added font-display: swap;)

LisaFC · 2021-05-05T15:44:01Z

This is great, thanks! I'll give it a proper look later.

chalin · 2021-08-09T15:34:22Z

layouts/partials/head.html

 {{ end -}}

-{{ if and hugo.IsProduction (ne $outputFormat "print") -}}
+{{ if and (eq (getenv "HUGO_ENV") "production") (ne $outputFormat "print") -}}


@jcabak - why were the changes introduced by #413 reverted here?
@LisaFC - maybe this is the case of #611?

IMHO we should be using hugo.IsProduction, not checking environment variable values -- in particular since a production build can be requested using the -e flag command line flag.

/cc @nate-double-u

I'm assuming that this (retrograde) change was accidental, but I could be wrong -- and thanks for clarifying the situation @jcabak when you have a minute. Let's carry the discussion over to #652.

Honestly I am surprised that I changed this line, you have right @chalin it was accidental. I appreciate your vigilance 🤠

Thanks for promptly confirming @jcabak. FYI, #653 reintroduces use of hugo.IsProduction.

chalin · 2021-08-09T15:35:06Z

layouts/partials/head.html

-{{ if hugo.IsProduction }}
+{{ if eq (getenv "HUGO_ENV") "production" }}


Ditto - https://github.com/google/docsy/pull/540/files#r685301715

chalin · 2021-09-18T13:12:09Z

layouts/partials/head.html

+  src="https://unpkg.com/lunr@2.3.8/lunr.min.js"
+  integrity="sha384-vRQ9bDyE0Wnu+lMfm57BlYLO0/XauFuKpVsZPs7KEDwYKktWi5+Kz3MP8++DFlRY"


Hi again @jcabak. Why was lunr's version retrograded?

Could it be that, aside from the addition of <meta name="description" ...>, every other change in this file is because you were working from an old version of the file (including the removal of the shortcodes.css link below)?

For some context, see https://github.com/google/docsy/pull/540/files#r685339766.

/cc @LisaFC @nate-double-u

chalin · 2021-09-18T13:15:28Z

layouts/partials/pager.html

@@ -1,7 +1,7 @@
-<ul class="list-unstyled d-flex justify-content-between align-items-center mb-0 pt-5 d-print-none">


@jcabak - what the removal of d-print-none voluntary? By doing so you're reverting #484.

Sorry @chalin, I don't know why I made mistakes, especially with removal stylesheet for tabbed pane and d-print-none. It shouldn't be like that.

chalin · 2021-09-18T13:18:54Z

layouts/partials/head.html

 <link rel="stylesheet" href="{{ "/css/prism.css" | relURL }}"/>
 {{ end }}
-<!-- stylesheet for tabbed pane -->
-<link rel="stylesheet" href="{{ "/css/shortcodes.css" | relURL }}"/>


@jcabak - by removing this link you're reverting #532. Was this intentional?

Investigating further, it seems that this was by mistake, but I'd appreciate a confirmation @jcabak.

I've created #694 to track this issue.

LisaFC · 2021-09-20T09:38:25Z

Thanks for noticing this @chalin! Waiting to hear from @jcabak about the removed CSS, it does look like an error (possibly by editing an old version of the file?) to me.

chalin · 2021-10-06T21:00:21Z

@jcabak - any updates?

chalin · 2021-10-07T14:21:36Z

Sorry @chalin, I don't know why I made mistakes, especially with removal stylesheet for tabbed pane and d-print-none. It shouldn't be like that.

No worries. As I mentioned to @LisaFC elsewhere, me might need to discuss & revisit the PR submission and review process to avoid such unintended consequences (e.g., by using squash-merges and requiring that PRs be submitted against the most recent HEAD).

Thanks for confirming! Now we can move forward with fixes.

LisaFC · 2021-10-07T14:27:05Z

Thanks so much @chalin for raising this and looking in to what happened!

And agree, we should look at what we can do to ensure that PRs aren't against an outdated version of the theme, particularly if they're large/complex. @emckean, did you have some/any ideas?

emckean · 2021-10-07T16:15:30Z

yes! I'm working on setting up some visual regression testing against docsy-example (with Percy: emckean/docsy-example#2), so that we can see more easily when something breaks. But setting some PR rules as @chalin suggested is a great idea! I have some time blocked for this tomorrow, let me see if I can push this to the top of the list ...

Accessibility and SEO improvements

b858d6c

LisaFC merged commit 604e27f into google:master May 5, 2021

chalin reviewed Aug 9, 2021

View reviewed changes

This was referenced Aug 9, 2021

Check hugo.Environment == “production” rather than HUGO_ENV=production environment variable #611

Closed

Changes introducing hugo.IsProduction were overwritten #652

Closed

Use hugo.IsProduction instead of checking HUGO_ENV #653

Merged

aimeeu mentioned this pull request Aug 18, 2021

FIX: meta description tag creation in head.html (SEO) #662

Merged

chalin reviewed Sep 18, 2021

View reviewed changes

This was referenced Sep 18, 2021

Card- & tab-group styling is no longer included #694

Open

Pager whitespace fix #696

Merged

This was referenced Oct 15, 2021

Process change suggestion: squash-and-merge PRs #725

Open

Add golden tests #726

Open

SayakMukhopadhyay mentioned this pull request Dec 23, 2024

Upgrade website to Docsy 0.5.1 kubernetes/contributor-site#531

Merged

		{{ if hugo.IsProduction }}
		{{ if eq (getenv "HUGO_ENV") "production" }}

		src="https://unpkg.com/lunr@2.3.8/lunr.min.js"
		integrity="sha384-vRQ9bDyE0Wnu+lMfm57BlYLO0/XauFuKpVsZPs7KEDwYKktWi5+Kz3MP8++DFlRY"

		@@ -1,7 +1,7 @@
		<ul class="list-unstyled d-flex justify-content-between align-items-center mb-0 pt-5 d-print-none">

Conversation

jcabak commented May 2, 2021

Uh oh!

LisaFC commented May 5, 2021

Uh oh!

chalin Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcabak Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chalin Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LisaFC commented Sep 20, 2021

Uh oh!

chalin commented Oct 6, 2021

Uh oh!

chalin commented Oct 7, 2021

Uh oh!

LisaFC commented Oct 7, 2021

Uh oh!

emckean commented Oct 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chalin Aug 9, 2021 •

edited

Loading

jcabak Aug 9, 2021 •

edited

Loading

chalin Aug 9, 2021 •

edited

Loading