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

Added hostname to og:url to include top level domain #7563

Merged
merged 12 commits into from
May 29, 2024
Merged

Conversation

nick-mon1
Copy link
Contributor

@nick-mon1 nick-mon1 commented May 14, 2024

Summary

Adds top level host name to og:url meta tags. Closes #7168.

Preview

Link to Preview

Solution

When viewing the og:url value on the live site, only the file pathname (/communities/) is displayed instead of https://www.digital.gov/communities/.

By setting a hostName params in the config.yml we can set the base host name.

Note

og:url path will be different for each environment

production
{{ .Permalink }} will only return the file path and not the host name:
https://digital.gov/resources/an-introduction-to-domain-management

cloud pages preview
{{ .Permalink }} returns the preview path and branch name with the file path:
https://digital.gov/preview/gsa/digitalgov.gov/nl-fix-og-url/resources/an-introduction-to-domain-management

localhost
{{ .Permalink }} will return localhost:1313 with the file path:
https://digital.gov/localhost:1313/resources/an-introduction-to-domain-management

How To Test

  1. Visit any page
  2. Open developer tools and search for og:url
  3. Should see https://digital.gov/preview/gsa/digitalgov.gov/nl-fix-og-url/resources/an-introduction-to-domain-management
  4. In production this will resolve to https://digital.gov/resources/an-introduction-to-domain-management

Dev Checklist

  • PR has correct labels
  • A11y testing (voice over testing, meets WCAG, run axe tools)
  • Code is formatted properly

Copy link

🔍 Preview in Federalist

@nick-mon1 nick-mon1 self-assigned this May 14, 2024
RileySeaburg
RileySeaburg previously approved these changes May 14, 2024
Copy link
Member

@RileySeaburg RileySeaburg left a comment

Choose a reason for hiding this comment

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

LGTM

@nick-mon1 nick-mon1 added the Content: General site For site specific pages (About, Subscribe, Host and event, Write for us...) label May 15, 2024
@nick-mon1 nick-mon1 marked this pull request as ready for review May 15, 2024 14:45
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

@nick-mon1 I tested locally and in preview. Inner pages don't seem to generate the correct link.

I noticed the baseURL is / in

baseURL: "/"

Changing it to https://digital.gov/ makes permalinks work as expected and follows the guidance here - Configure Hugo | Hugo.

We should investigate why we need the / and if this is a legacy config option. Alternatively, we could try a workaround like making the permalink absolute.

Example

<meta
    property="og:url"
+    content="{{- .Permalink | absURL -}}"
  />

<meta property="og:url" content="{{- .Permalink -}}" />
<meta
property="og:url"
content="{{ .Site.Params.hostName }}{{- .Permalink -}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Communities

Returns og:url value of https://digital.gov//localhost:1313/communities/.

image

In Cloud Pages preview I see a link that leads to 404:

https://federalist-466b7d92-5da1-4208-974f-d61fd4348571.sites.pages.cloud.gov/preview/gsa/digitalgov.gov/nl-fix-og-url/communities/

It should be:

https://federalist-466b7d92-5da1-4208-974f-d61fd4348571.sites.pages.cloud.gov/preview/gsa/digitalgov.gov/nl-fix-og-url/communities/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mejiaj Both of the URL's are the same in the examples above. For some reason I do not see https://www.digital.gov set on my local, still uses localhost:1313, could be a caching issue with your code suggestions.

  1. Updated baseURL: https://www.digital.gov in config.yml
  2. And set:
<meta property="og:url" content="{{- .Permalink | absURL -}}"/>

Copy link
Member

@RileySeaburg RileySeaburg left a comment

Choose a reason for hiding this comment

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

URL not displaying as expected.
Previous code change worked as expected.

@nick-mon1
Copy link
Contributor Author

Explanation

  1. Set the baseURL: "https://digital.gov" in the config.yml for local development. See comment in hugo support.
    1. When running hugo locally, the baseURL will be replaced with localhost:1313 for links to work
    2. When running in production, it will be replaced with https://digital.gov
    3. When baseURL: "/" or baseURL: "" (blank) it will not generate full paths in production which is what is happening currently
  2. Follow the hugo convention of using {{ .Permalink | absURL }} for setting og:url path.
  3. Optional: set hugo serve -b https://digital.gov for testing/production environment if above fix doesn't work, this is the recommended suggestion for dealing with env configuration

Next Steps

@RileySeaburg the current URL's we are seeing are due to cloud.pages configuration, but they should resolve when we merge to main. If not, we should do step 3 above in a follow up PR if this fix doesn't work.

@RileySeaburg
Copy link
Member

I think we'll need to reconfigure cloud.gov because otherwise, I'm not sure why it would show the URL like that.
Local changes look great. Approving.

@mejiaj
Copy link
Contributor

mejiaj commented May 28, 2024

  1. Optional: set hugo serve -b https://digital.gov for testing/production environment if above fix doesn't work, this is the recommended suggestion for dealing with env configuration

@nick-mon1 @RileySeaburg Cloud pages has some default environment variables. Could we use that to target the preview builds?

@RileySeaburg
Copy link
Member

  1. Optional: set hugo serve -b https://digital.gov for testing/production environment if above fix doesn't work, this is the recommended suggestion for dealing with env configuration

@nick-mon1 @RileySeaburg Cloud pages has some default environment variables. Could we use that to target the preview builds?

@mejiaj Yes, we can.

@nick-mon1 not sure if you want to just do that or create a task and make this dependent on that task.
It might be worth creating a new task to review the environment variables in cloud.gov and optimize them.

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Looks good and I don't see the notice bar banner locally anymore.

@ToniBonittoGSA ToniBonittoGSA merged commit 57645f3 into main May 29, 2024
8 checks passed
@ToniBonittoGSA ToniBonittoGSA deleted the nl-fix-og-url branch May 29, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: General site For site specific pages (About, Subscribe, Host and event, Write for us...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: og:url doesn't have complete top-level domain
4 participants