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

MDX files for metadata for all pages without manifests #34

Open
wants to merge 10 commits into
base: bug/DEL-108-title-and-description
Choose a base branch
from

Conversation

funkydunc
Copy link
Contributor

@funkydunc funkydunc commented Jan 20, 2025

This PR is based on the branch for #32 so should be merged first.

Provides a consistent mechanism for customising metadata on each site page.
As we know, the Exhibition, Collection, Object detail pages will get the metadata from the meta.json file, which has been populated from the Manifest. And the Publication detail page and the About page use an mdx file.

This PR makes the rest of the pages consistent with that mechanism for supplying metadata. It adds MDX files with title, description, image for:

  • Home, About, Search
  • Exhibitions, Collections, Publications listing pages

title is required

If description or image is not supplied in these files, it will use the defaults. Default description is in the lang file, and the default image is the image provided in #32

Here are screenshots using a custom description and image. Custom values were tested locally but not pushed, followed by the initial use case: using default description and image as fallback.

Note the contents of the social share preview in each case.

CUSTOM EXAMPLES:

custom mdx code (used to test but not committed)
custom mdx src

Home custom example
Home custom

Search custom example
Search custom

Exhibitions custom example
Exhibitions custom

USING DEFAULTS:
Home default
Home default

About default
About

Search default
Search default

Exhibitions default
Exhibitions default

@funkydunc funkydunc changed the base branch from main to bug/DEL-108-title-and-description January 20, 2025 13:23
@stephenwf stephenwf added the deploy-preview PRs tagged with this will deploy a Netlify preview URL label Jan 21, 2025
Copy link

Netlify output

Static site (preview): https://deploy-preview-34--heritage-tudelft.netlify.app
IIIF (preview): https://deploy-preview-34--delft-iiif.netlify.app
IIIF Collection (preview): Theseus

@funkydunc
Copy link
Contributor Author

Good morning @sammeltassen
In response to your comment on #32 :

It would make more sense to me to add the descriptions of the static pages in the frontmatter of the mdx source file and if this is missing use the default description. Would that be possible?

This PR allows all the titles and descriptions on the static pages to be controlled by mdx files. If you could please review this to let me know if you are happy with the solution, that would be great thanks! There is a more detailed description and screenshots above.

@sammeltassen
Copy link
Collaborator

Thank you @funkydunc this is great! Two comments/suggestions:

  • Would it make sense to use title/description/image of the homepage as defaults?
  • Where's the setting for the site title that is used for the breadcrumb? TU Delft Academic Heritage is used but this is not in the translation file.
  • The breadcrumb should be removed from the homepage title: (it's now: Academic Heritage, History and Art | TU Delft Academic Heritage).

@sammeltassen
Copy link
Collaborator

Another remark: rather than pushing the default image to the repo I'd prefer to specify it in the homepage mdx and use a DLCS thumbnail url. So this would be the contents of both home.mdx files:

--- # en
title: Academic Heritage, History and Art
path: "/"
description: Explore the history of Delft University of Technology and the Special Collections of TU Delft Library
image: https://dlc.services/thumbs/7/13/00a1a6c3-25cb-4873-b570-2a77205852c9/full/683,1024/0/default.jpg
---
--- # nl
title: Academisch Erfgoed, Geschiedenis en Kunst
path: "/"
description: Verken de geschiedenis van de TU Delft en de bijzondere collecties van de TU Delft Library
image: https://dlc.services/thumbs/7/13/00a1a6c3-25cb-4873-b570-2a77205852c9/full/683,1024/0/default.jpg
---

With all these values used as defaults across the site.

@funkydunc
Copy link
Contributor Author

funkydunc commented Jan 22, 2025

Thanks for the feedback @sammeltassen

Would it make sense to use title/description/image of the homepage as defaults?

I can do that. The function that gets the mdx for the current page can return the home values as defaults, if the values are not set in the current page mdx.

Where's the setting for the site title that is used for the breadcrumb? TU Delft Academic Heritage is used but this is not in the translation file.

Currently here

return `TU Delft ${t("Academic Heritage")}`;

I can change that to use a new independent entry in the translation files t('TU Delft Academic Heritage).

The breadcrumb should be removed from the homepage title: (it's now: Academic Heritage, History and Art | TU Delft Academic Heritage).

OK

@sammeltassen
Copy link
Collaborator

sammeltassen commented Jan 22, 2025

I see the problem of extracting the width and height for the default image if specified as an url. Possible solutions:

  • Linking to the info.json of the thumbnail service instead (and extracting the size from there)
  • The width and height can be extracted from the url (but as specifying both width and height is not required by the thumb service this is prone to mistakes/errors)
  • Adding width and height manually in the frontmatter

@sammeltassen
Copy link
Collaborator

@funkydunc You can use the homepage title for the breadcrumb or perhaps add a separate short-title property for that to the frontmatter?

@funkydunc
Copy link
Contributor Author

Hi @sammeltassen I've pushed an update:

  • It uses the Home title, description, image as defaults for all other pages, if any of those properties are not defined either in the page mdx or the manifest.
  • mdx 'Page' and 'Publication' now also have optional imageHeight and imageWidth. For site pages, if they have their own image defined in the mdx or in the manifest, it also will look for a image height and width in the manifest or mdx. If there is no image defined it uses the Home image, and the Home imageWidth and imageHeight. So you should put the imageHeight and imageWidth into the Publication files.
  • The home page titles have been amended as you requested,

Can you please clarify: On the other pages, for the part of the title breadcrumb after the main title, do you want that to be the same as the Home title or "TU Delft Academic Heritage" e.g. Voices of Women in Science | Academic Heritage, History and Art or ``Voices of Women in Science | TU Delft Academic Heritage`

TU Delft Academic Heritage has been added to the lang files. If you want to use the Home title in the breadcrumb then we can remove TU Delft Academic Heritage from the lang files.

@sammeltassen
Copy link
Collaborator

@funkydunc Thank you for these changes. You can remove TU Delft Academic Heritage from the language files and use the main title for the breadcrumb.

The dimensions for the default image are:

imageWidth: 683
imageHeight: 1024

@funkydunc
Copy link
Contributor Author

@sammeltassen what about in the <og:site_name> is that ok to be the default title too, or do you want to keep the reference to TU Delft Academic Heritage there, as its the name of the site? Thanks

@sammeltassen
Copy link
Collaborator

@funkydunc Yes that can also make use of the default title

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-preview PRs tagged with this will deploy a Netlify preview URL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants