posit-dev / brand-yml

Unified branding with a single yaml file.
https://posit-dev.github.io/brand-yml/
MIT License
1 stars 0 forks source link

`logo` images should support alt text #19

Open gadenbuie opened 1 month ago

gadenbuie commented 1 month ago

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

gadenbuie commented 1 month ago

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

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

Quarto now has a brand-logo-resource schema:

https://github.com/quarto-dev/quarto-cli/blob/5c83513a78ded63ed3f6faaf6a7ecba5f1ec2ce0/src/resources/schema/definitions.yml#L2518-L2529

gadenbuie commented 3 days ago

@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 commented 3 days ago

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

gadenbuie commented 2 days ago

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