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

logo images should support alt text #19

Open
gadenbuie opened this issue Sep 11, 2024 — with Slack · 7 comments
Open

logo images should support alt text #19

gadenbuie opened this issue Sep 11, 2024 — with Slack · 7 comments

Comments

Copy link
Collaborator

We should allow logo images to have alt text information attached to them.

@gadenbuie
Copy link
Collaborator Author

gadenbuie commented Sep 11, 2024

The University of South Carolina is a really good test case: https://sc.edu/about/offices_and_divisions/digital-accessibility/toolbox/best_practices/alternative_text/logo-alt-text/index.php

logo:
  images:
    logo: grid_uofsc_formal.png
    seal:
      image: grid_seal.png
      alt: University of South Carolina seal

  small:
    light: grid-usc-horiontal-rev-on-white.svg
    dark: grid-usc-horiontal-rev-g-on-black.svg
    alt: University of South Carolina logo
  medium:
    image: logo
    alt: University of South Carolina logo
  large: seal
  • image and alt are siblings and are used together in the same leaf node
  • alt is also a field of small, medium, and large
  • image is a generic term that stands in for "both light and dark variants"
  • Can't use image together with light and dark, it's either image OR light or dark.

In the next example, we define two logo.images and use them as light and dark variants.

logo:
  images:
    rev-on-white:
      image: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    rev-on-black:
      image: grid-usc-horiontal-rev-g-on-black.svg
      alt: University of South Carolina logo

  medium:
    light: rev-on-white
    dark: rev-on-black

Once parsed, we should internally use this structure:

logo:
  images:
    rev-on-white:
      image: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    rev-on-black:
      image: grid-usc-horiontal-rev-g-on-black.svg
      alt: University of South Carolina logo

  medium:
    light:
      image: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    dark:
      image: grid-usc-horiontal-rev-g-on-black.svg
      alt: University of South Carolina logo

In other words, regardless of how logo.small, logo.medium or logo.large are specified, we'll always cast them into this structure:

{size}:
  light:
    image: {image}
    alt: {alt}
  dark:
    image: {image}
    alt: {alt}

where specifying image or alt one level up would cast image and alt into the light/dark structure.

Input Parsed
logo:
  medium:
    image: logo
    alt: Logo alt text
logo:
  medium:
    light:
      image: logo.png
      alt: Logo alt text
    dark:
      image: logo.png
      alt: Logo alt text
logo:
  medium:
    light: logo-light.png
    alt: Logo alt text
logo:
  medium:
    light: 
      image: logo-light.png
      alt: Logo alt text
logo:
  medium:
    light: logo-light.png
    dark: 
      image: logo-dark.png
      alt: Dark logo alt text
    alt: Logo alt text
logo:
  medium:
    light: 
      image: logo-light.png
      alt: Logo alt text
    dark: 
      image: logo-dark.png
      alt: Dark logo alt text

That said, I'm tempted to simplify this slightly by having users give us image and alt as the keys of small, medium, and large...

small:
  image:
    light: logo-light.png
    dark: logo-dark.png
  alt: Dark logo alt text

but this gets weird because the idea that image and alt should go together leads to the following:

small:
  image:
    light: 
      image: logo-light.png
      alt: Light logo alt text
    dark: 
      image: logo-dark.png
      alt: Dark logo alt text

So I think we should support these variants:

small: string
medium:
  image: string
  alt: string
large:
  alt: string
  light: string # inherits alt
  dark:
    image: string
    alt: string
  # no image allowed because of light/dark

I'd really like to say that alt is only allowed under small, medium, and large, but that leaves ambiguity about how to specify alt under logo.images, which is a required feature.

logo
  images:
    logo-light:
      image: logo-light.png
      alt: Light logo alt text

  small:
    light: logo-light

Note that in the above, we've now introduced small.light.image.alt. Furthermore, that's the structure we'd want internally after parsing anyway, since the most consistent (and verbose) construction is

logo
  images:
    logo-light:
      image: logo-light.png
      alt: Light logo alt text

  small:
    light:
      image: logo-light.png
      alt: Light logo alt text

@cscheid
Copy link
Collaborator

cscheid commented Oct 7, 2024

FWIW, I think Quarto wants to support href: instead of (or in addition to) image:, because it's consistent with the rest of Quarto's YAML scehams.

@gadenbuie
Copy link
Collaborator Author

gadenbuie commented Oct 9, 2024

This is the general structure we'll go with, but with path instead of image for the logo path.

In the next example, we define two logo.images and use them as light and dark variants.

logo:
  images:
    logo: grid_uofsc_formal.png
    rev-on-white:
      path: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    rev-on-black:
      path: grid-usc-horiontal-rev-g-on-black.svg
      alt: University of South Carolina logo

  medium:
    light: rev-on-white
    dark: rev-on-black

Once parsed, we should internally use this structure:

logo:
  images:
    rev-on-white:
      path: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    rev-on-black:
      path: grid-usc-horiontal-rev-g-on-black.svg
      alt: University of South Carolina logo

  medium:
    light:
      path: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    dark:
      path: grid-usc-horiontal-rev-g-on-black.svg
      alt: University of South Carolina logo

cscheid added a commit to quarto-dev/quarto-cli that referenced this issue Oct 10, 2024
@cscheid
Copy link
Collaborator

cscheid commented Oct 10, 2024

@gadenbuie
Copy link
Collaborator Author

@cscheid To confirm I've read the schema change correctly, it seems like the Quarto change adds supports for brand-logo-resource in logo.images only, i.e. these are okay

# [Ex 1a]
logo:
  images:
    logo: grid_uofsc_formal.png
    rev-on-white:
      path: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo

and then small, medium and large can be either a key from logo.images or a light-dark each pointing to a key in images.

# [Ex 1b]
logo:
  small: logo
  medium:
    light: rev-on-white
    dark: rev-on-black

But logo.* or light/dark aren't allowed to directly take a brand-logo-resource, i.e. Quarto's YAML validation would flag these:

# [Ex 2]
logo:
  small:
    path: grid_uofsc_formal.png
    alt: Logo of USC
  medium:
    light:
      path: grid-usc-horiontal-rev-on-white.svg
      alt: University of South Carolina logo
    dark: ...

If I've understood correctly, I'm not conceptually opposed to this restriction, but it's a little awkward in Python because even if we don't let users write Brand YAML "by hand" as in Ex 2, I'll still have to convert the restricted form (Ex 1a + 1b) into the full form (resembling Ex 2) once parsed.

Because of that, I'm not going to be able to enforce the restriction when reading a _brand.yml into Python, so we'll end up with part of the YAML structure that's undocumented and valid but disallowed by Quarto's linting.

Given the amount of context sharing this project introduces, I'd prefer to make the brand schema as explicit as possible. Which means I'd like to include Ex 2 in the schema, even if we don't advertise it much in docs and examples. What do you think?

@cscheid
Copy link
Collaborator

cscheid commented Oct 16, 2024

I think your request is something that @gordonwoodhull suggested we do already. I'll make the change and I'll report back.

@gadenbuie
Copy link
Collaborator Author

gadenbuie commented Oct 16, 2024

Next wrinkle: we currently allow logo: brand-yaml.png, i.e. logo can be a path to a single value. Should we:

  1. Be permissive and let logo be a string, a BrandLogoResource ({"path": ..., "alt": ...}), or a full BrandLogo instance?
  2. Keep the simple version without alt text, i.e. logo: str | BrandLogo | None.
  3. Drop support for the simple case and use logo: BrandLogo | None?

Thoughts @cscheid @gordonwoodhull?

Option 3 is the most consistent in terms of how other fields work. No other top-level brand field lets you do key: {string}, but it makes the base case of setting small, medium and large to the same image a bit more complicated for the user:

logo: brand-yaml.png

# alt specifying directly...
logo:
  small: brand-yaml.png
  medium: brand-yaml.png
  large: brand-yaml.png

# alt using images...
logo:
  images:
    brand-yaml: brand-yaml.png
  small: brand-yaml
  medium: brand-yaml
  large: brand-yaml

There's also useful signal in having logo: brand-yaml.png to say that this is catch-all logo and not specifically designed for a designed size. Option 1 means users could do either of these:

logo: brand-yaml.png

logo:
  path: brand-yaml.png
  alt: The Brand YAML logo

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

No branches or pull requests

2 participants