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

refactor label.js out of existence #452

Open
3 of 6 tasks
pkra opened this issue Jul 4, 2024 · 8 comments
Open
3 of 6 tasks

refactor label.js out of existence #452

pkra opened this issue Jul 4, 2024 · 8 comments
Assignees
Milestone

Comments

@pkra
Copy link
Member

pkra commented Jul 4, 2024

From #451 (comment)

Looking more at label.js, I think I'd rather get rid of that entirely and move its logic into the various parent pieces -- label and title are just too messy.

  • case: fn 40f8899
  • case: ref b5aebe2
  • case: fig etc fe5bde5
  • case: disp-formula-group
  • case: statement
  • complex cases
@pkra pkra added this to the 2024 milestone Jul 4, 2024
@pkra pkra self-assigned this Jul 4, 2024
@pkra
Copy link
Member Author

pkra commented Aug 9, 2024

Random thought: this might end up somewhat differently - label and title could just create spans with respective data-ams-doc and parents create wrappers and recurse title and label with those.

pkra added a commit that referenced this issue Aug 13, 2024
Moves label handling to ref.js.
- label.js
  - remove case "ref" (i.e., passThrough)
- ref.js
  - replace recurseTheDom with passThrough

Part of #452
@pkra pkra mentioned this issue Aug 13, 2024
pkra added a commit that referenced this issue Aug 13, 2024
Moves caption.js into fig.js and simplifies label.js
- transformer.js
  - remove caption.js import /integration
- (re)move caption.js
  - now handled in fig.js
- fig.js
  - if label or caption, create figcaption
  - if label, create strong element (sames as caption.js)
  - if caption, pass through with figcaption
- label.js
  - remove caption handling

Part of #452
pkra added a commit that referenced this issue Aug 13, 2024
Moves label logic from label.js to fn.js

- fn.js
  - add label creation logic from label.js
- label.js
  - remove fn handling

Part of #452
@pkra
Copy link
Member Author

pkra commented Aug 13, 2024

fn is somewhat of a wrench - it injects a sup element inside the "typical" label span. This will make the idea from the previous comment a bit harder - unless we move sup-creation downstream (or even upstream).

@pkra
Copy link
Member Author

pkra commented Aug 13, 2024

For the record, the tags of label parents:

toc-entry
sec
book-app-group => sec
book-app => sec
app
disp-formula-group
fn
statement
fig
fig-group => fig
table-wrap => fig
table-wrap-group => fig
ref
secheading

@pkra
Copy link
Member Author

pkra commented Aug 13, 2024

disp-formula doesn't really need a figcaption wrapper around the label (or a figure for that matter).

@pkra pkra modified the milestones: 2024, 2025 Jan 7, 2025
@pkra
Copy link
Member Author

pkra commented Jan 8, 2025

To quote #457 (comment)

Regarding the OP, I still agree that this is similar to the label.js situation. Ideally, attrib.js would just create a span (and importantly: add a data attribute which is currently missing). Then we'd need a good pattern to smoothly handle the other cases. The latter seems like the main challenge.

Similarly, ideally, we'd have label and title just create spans with their respective data attributes and find a smooth pattern to handle any re-organization in the parents.

@pkra
Copy link
Member Author

pkra commented Jan 8, 2025

Similarly, ideally, we'd have label and title just create spans with their respective data attributes and find a smooth pattern to handle any re-organization in the parents.

On a related note, in fig.js, we map label to strong (via passthrough); that's really not necessary, semantically speaking but it would be a breaking change. (A generic label mapping would still need to be moved/redirected to the figcaption.)

@pkra
Copy link
Member Author

pkra commented Jan 10, 2025

After too much additional research, it's tempting to "just" reduce this to the case where we generate the heading. But that would be lazy - I'd prefer to reflect the complexity of the content here so there are no fewer surprises down the line.

Just to recap the parent elements

// fig-likes are already done
'fig',
'fig-group',
'table-wrap-group',
'table-wrap',

// overlap with fig-likes?
'statement',
'disp-formula-group',

// "just a label/title"
'fn',
'toc-entry',
'ref',

// tricky b/c pass-through - metadata and sometimes in sec-likes
'title-group',

// sec-likes where we want headings.
'sec',
'ref-list',
'book-app',
'notes',
'secheading',
'abstract',
'named-book-part-body',
'app',
'book-app-group',
'ack',

@pkra
Copy link
Member Author

pkra commented Jan 10, 2025

An update

  • statement and disp-formula group were fairly straight forward, based on fig.js .
  • fn, toc-entry, and ref already handle label/title on their own (but might benefit from refactoring once we have reduced label.js to just generating the label/title span)
  • 'named-book-part-body' is only in an ancient book - the title should nowadays be in a book-part-meta title-group

This leaves

  • sec, ref-list, notes, secheading, abstract and app seem fairly straight forward (just re-using what's in label.js right now)
  • title-group seems tricky. It is passed-through right now. Outside metadata (which is handled) and toc (which is easy) it shows up in book-part-meta (another pass-through).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant