Skip to content

[Hotfix]: Make alttext work for cover image and when pandocbounded is not present#405

Merged
Schiano-NOAA merged 5 commits intomainfrom
hotfix-a11y-alttext
Dec 19, 2025
Merged

[Hotfix]: Make alttext work for cover image and when pandocbounded is not present#405
Schiano-NOAA merged 5 commits intomainfrom
hotfix-a11y-alttext

Conversation

@Schiano-NOAA
Copy link
Copy Markdown
Collaborator

What is the feature?

  • Adjusts the method from pdftooltip to pandocbounded for alt text in the cover page
  • Instead of removing lines that do not have pandocbounded or tooltip, the function just removes the line for the artifact since this is the only known line there that has alt text

How have you implemented the solution?

  • change format to the alt text in the cover page to match that in add_alttext
  • Remove the loop that id's which images don't have pandocbounded and instead id the line that has the artifact

Does the PR impact any other area of the project, maybe another repo?

  • no

@Schiano-NOAA Schiano-NOAA marked this pull request as ready for review December 19, 2025 18:56
@Schiano-NOAA Schiano-NOAA requested review from Copilot and sbreitbart-NOAA and removed request for sbreitbart-NOAA December 19, 2025 18:57
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 19, 2025

Checklist

  • PR base branch is accurate
  • Is the code concise?
  • Comments are clear and useful.
  • Can you remove or combine any arguments?
  • Do argument contain defaults (if appliable)?
  • Code is documented and example provided (Roxygen).
  • Did you make a test (testthat)?
  • Was this tested under multiple scenarios?
  • Did you run devtools::check()?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This hotfix addresses alt text handling for cover images and figures that don't have the pandocbounded wrapper in LaTeX documents. The changes unify the alt text format and simplify the logic for identifying which figure lines need alt text processing.

  • Updated cover page alt text from pdftooltip format to pandocbounded format with inline alt= attribute
  • Replaced loop-based filtering logic with direct artifact line removal to handle figures without pandocbounded

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
R/utils_tex.R Modified cover page alt text generation to use pandocbounded with inline alt= attribute instead of pdftooltip wrapper format
R/add_alttext.r Simplified figure line filtering by directly removing artifact line instead of looping through all lines to check for pandocbounded

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Copy Markdown
Contributor

Code Metrics Report

Coverage Code to Test Ratio Test Execution Time
47.7% 1:0.1 4m16s

Code coverage of files in pull request scope (31.3%)

Files Coverage
R/add_alttext.r 0.0%
R/utils_tex.R 58.3%

Reported by octocov

Copy link
Copy Markdown
Collaborator

@sbreitbart-NOAA sbreitbart-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good! I'm not sure why only the mac R Cmd check is failing, but that happened to me last week and eventually it disappeared.

@Schiano-NOAA
Copy link
Copy Markdown
Collaborator Author

@sbreitbart-NOAA great! I'm going to push a check for the artifact then merge after checks

@Schiano-NOAA Schiano-NOAA merged commit 3eb1d5b into main Dec 19, 2025
11 of 13 checks passed
@Schiano-NOAA Schiano-NOAA deleted the hotfix-a11y-alttext branch December 19, 2025 21:09
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