Figure identifiers must be lowercase, no spaces, etc, for scroll-to-lightbox-slide to work

cbutcosk commented 9 months ago

Before proceeding, make sure there isn’t an existing issue for this bug.

[X] I have searched the existing issues and determined this is a new bug.

Expected Behavior

Adding images to figures.yaml that have uppercase letters, periods, or underscores I expect clicking on an image in the essay to still bring up that figure's slide in the lightbox modal.

Or, if that's not going to work I expect it to break on build and tell me why. And I expect any constraints on figure identifiers to be documented. :)

Actual Behavior

The figures are processed fine and appear in the essay but clicking on a figure brings you to the start of the lightbox rather than the slide of the figure you clicked. The only way I was able to get the spec data below to build was to replace figure identifiers' uppercase chars with lowercase and all periods / . with dashes (underscores as replacements did not work).

Note that the quire docs use a lot of figures with '.' in them!

Steps to Reproduce

With this drawn-from-real data:

  - id: "XXXX.1111.22"
    src: figures/XXXX_1111_22_FPO.jpg
    caption: "<cite>Work Title</cite>, Work Cataloguing Details. Various dimensions."
    credit: "ZZZZ © PHOTO CREDIT TK."
    zoom: true

  - id: "YYY-33333"
    src: figures/YYY-33333.jpg
    caption: "<cite>Another Work Title</cite>, 1980. Detailed cataloguing, Various dimensions."
    credit: "ZZZZ © PHOTO CREDIT TK."
    zoom: true

Version Numbers

[CLI] Command 'info' called with options { debug: true } [test-latest] quire-cli 1.0.0-rc.10 quire-11ty 1.0.0-rc.14 starter https://github.com/thegetty/quire-starter-default@2.6.0 [System] quire-cli 1.0.0-rc.10 node v18.17.0 npm 9.6.7 os Darwin 22.6.0

Web Browser

Platform: macOS, chrome: latest, safari: 17.0

Relevant Terminal/Shell Output

No response

Supporting Information

No response

Erin-Cecele commented 9 months ago

Thank you, @cbutcosk, for bringing this to our attention and noting the discrepancy in the documentation. We'll take a closer look and keep you updated.

Erin-Cecele commented 8 months ago

Hi @cbutcosk I was able to recreate these errors via testing and actually couldn't even get the preview to work. I noticed the documentation is particularly wrong, as it features examples using periods! So it appears ids have become more sensitive since we switched from Hugo to 11ty. This one slid past us, as we only ever use dashes in ids at Getty. Thank you for finding and reporting!

I will absolutely update the documentation to clarify. Can you see any reason why we should push to make ids less sensitive or is updating the documentation sufficient to address this issue?

cbutcosk commented 8 months ago

@Erin-Cecele Great, thanks for updating! You can close this issue for now I think--although updating the code to be more flexible around figure id constraints would be a nice to have ("structure your ids so they're document query selectors" is... weird) it's downstream of some other gotta-fix-that's in the quire runtime.

Erin-Cecele commented 8 months ago

Thanks @cbutcosk. Ideally, Quire's handling of ids would be analogous to the rules for id values in HTML: ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods ("."). But yes this is a bit downstream from some of the more pressing fixes on the docket.

As a side note, we also discovered that Quire is converting any id used (one coming from figures.yaml or just an id that was added to a random element on the page) to be only lowercase; and underscores, colons and periods are converted to hyphens. This appears to be a separate but most likely related issue, so stay tuned as, we may be opening another ticket for that.

Erin-Cecele commented 8 months ago

We are backlogging this issue as we eventually need to determine what subset of valid ids will work across JavaScript, HTML, and CSS.

cbutcosk commented 8 months ago

@Erin-Cecele Hah just the issue I mean. On the one hand whatever bugs that relaxing the slugifying of ids introduces need fixed and on the other hand / as a result the js runtime needs some event handling cleanup and more pub data intelligence so it does less direct querySelector-ing of params.

thegetty / quire