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

Tagging issues 777 and 778 #1607

Merged
merged 14 commits into from
Jan 16, 2025
Merged

Tagging issues 777 and 778 #1607

merged 14 commits into from
Jan 16, 2025

Conversation

u-fischer
Copy link
Member

Status of pull request

  • Ready to merge

Checklist of required changes before merge will be approved

  • Test file(s) added
  • Version and date string updated in changed source files
  • Relevant \changes entries in source included
  • Relevant changes.txt updated
  • [n/a] Rollback provided (if necessary)?
  • [n/a ] ltnewsX.tex (and/or latexchanges.tex) updated

@u-fischer
Copy link
Member Author

@FrankMittelbach @davidcarlisle I have now changed the taggging of presentation tables to use "normal" tags and an ARIA attribute. This works better with derivation. OK?

@davidcarlisle
Copy link
Member

Looks good to me

Copy link
Member

@FrankMittelbach FrankMittelbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see question about plug settings

Comment on lines +364 to +367
\AddToHook{cmd/frontmatter/before}{\par\@@_sec_end:n{-10}}
\AddToHook{cmd/mainmatter/before} {\par\@@_sec_end:n{-10}}
\AddToHook{cmd/backmatter/before} {\par\@@_sec_end:n{-10}}
\AddToHook{cmd/appendix/before} {\par\@@_sec_end:n{-10}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better would be to move the \par command into those comands eventually (to ensure that they always behave identically (with or without tagging).

Also (not checked) but isn't \@@_sec_end:n{-10} a related command and should be in a socket rather than a hook?

Doesn't have to be done now, but there should be a TODO if this is something we should cleanup at some point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if a \par inside \@@_sec_end:n is always wanted that's why I added if here explicitly (In the other cases the command is currently is used inside the sectioning commands in vmode, so a \par inside the command is not needed).

I'm not sure what you mean by sockets. \mainmatter etc are defined by the classes, so we can not simply redefine them to add a socket but have to use a generic hook. The problem was that the hook is before the \clearpage and so not necessarly in vmode.

Comment on lines 1060 to 1064
\__tag_tbl_enable:
\AssignSocketPlug{tagsupport/tbl/hmode/begin}{LayoutTable}
\AssignSocketPlug{tagsupport/tbl/vmode/begin}{LayoutTable}
\clist_clear:N \l_@@_header_rows_clist
\clist_clear:N \l_@@_header_columns_clist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I go from presentation back to table/tagging=on it should probably reset the plugs or not? If so the plugs have to be set also above, as this this might happen on the same group level.

If my assumption is correct, then this also needs a test case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\__tag_tbl_enable: sets all sockets, including the begin sockets to the defaults. So the presentation mode only has to overwrite the two.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and there is a testfile (table-015-presentation in testfiles-table-luatex) which tests the switch from presentation back to normal tagging in one group.

@u-fischer
Copy link
Member Author

@FrankMittelbach, @davidcarlisle I now added both options, (aria-role and div) so I think that is good to merge?

Copy link
Member

@FrankMittelbach FrankMittelbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some suggested changes and a question

required/latex-lab/latex-lab-table.dtx Outdated Show resolved Hide resolved
% (p/m/b). In such cases the second option works better: it keeps the tagging code active,
% but adds an ARIA-role as attribute. When deriving to html this gives the best result
% as it keeps the layout.
% The last option changes the tag names and creates a number of DIV structures.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o ... creates simply a number of ...

@@ -228,7 +234,7 @@
% \item Not every table should be tagged as a Table structure, often they are
% only used as layout help, e.g. to align authors in a title pages. In such uses
% the tagging of the table must be deactivated with \verb|\tagpdfsetup{table/tagging=false}|
% or \verb|\tagpdfsetup{table/tagging=presentation}|.
% or changed with \verb|\tagpdfsetup{table/tagging=presentation}|.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... to become a presentation table.

what about the DIV version?

required/latex-lab/latex-lab-table.dtx Outdated Show resolved Hide resolved
@u-fischer u-fischer merged commit 33f80ff into develop Jan 16, 2025
86 checks passed
@u-fischer u-fischer deleted the tagging-777-778 branch January 16, 2025 17:32
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

Successfully merging this pull request may close these issues.

3 participants