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

Consider providing a message/warning if fig.alt is not defined? #23

Closed
TimTaylor opened this issue Aug 23, 2024 · 3 comments
Closed

Consider providing a message/warning if fig.alt is not defined? #23

TimTaylor opened this issue Aug 23, 2024 · 3 comments

Comments

@TimTaylor
Copy link
Contributor

Would it be worth giving a little nudge via a message / warning if fig.alt is not provided for an image/plot within a document?

@yihui
Copy link
Owner

yihui commented Sep 6, 2024

I don't know. It's quite simple to implement, but I'm not sure if users would find the messages pesky. I can consider adding an option so users can opt-in to see such messages (e.g., those who do want to get reminded of missing alt text). Thoughts?

diff --git a/R/fuse.R b/R/fuse.R
index a7b9a58..ac4fbb2 100644
--- a/R/fuse.R
+++ b/R/fuse.R
@@ -679,7 +679,6 @@ fuse_code = function(x, blocks) {
   env = opts$fig.env; alt = opts$fig.alt; cap = opts$fig.cap
   att = if (is.null(att <- opts$attr.plot)) '' else paste0('{', att, '}')
   if (is.null(alt)) alt = cap
-  if (is.null(alt)) alt = ''
   p1 = Filter(function(x) !is_plot(x), res)
   p2 = Filter(is_plot, res)
   # get the relative path of the plot directory
@@ -690,6 +689,15 @@ fuse_code = function(x, blocks) {
 
   # recycle alt and attributes for all plots
   pn = length(unlist(p2))
+  if (pn && is.null(alt)) {
+    # reminder about missing alt text if this option is set to TRUE
+    if (getOption('litedown.fig.alt', FALSE)) message(
+      "\nPlease provide a 'fig.alt' option to the code chunk at ", get_loc(lab)
+    )
+    alt = ''
+  }
   alt = rep(alt, length.out = pn)
   att = rep(att, length.out = pn)
   # if figure caption is provided, merge all plots in one env

@TimTaylor
Copy link
Contributor Author

TimTaylor commented Sep 6, 2024

I like the option approach. It then also makes it very easy for you to switch the default (if there's a desire/need to) in future.

@yihui yihui closed this as completed in dfa5027 Sep 6, 2024
@yihui
Copy link
Owner

yihui commented Sep 6, 2024

Okay, I just committed the code above. Thanks!

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

No branches or pull requests

2 participants