Skip to content

fix: wrong object used in EJS to get logo-alt for sidebar#10254

Merged
cscheid merged 12 commits intoquarto-dev:mainfrom
mcanouil:fix/issue10251
Mar 5, 2025
Merged

fix: wrong object used in EJS to get logo-alt for sidebar#10254
cscheid merged 12 commits intoquarto-dev:mainfrom
mcanouil:fix/issue10251

Conversation

@mcanouil
Copy link
Copy Markdown
Collaborator

@mcanouil mcanouil commented Jul 8, 2024

Fixes #10251

This pull request fixes the issue where the logo-alt text was not being displayed in the sidebar. The problem was caused by an incorrect reference to the navbar['logo-alt'] variable in the EJS template for the website. The fix updates the template to use the correct object reference sidebar['logo-alt'] for the alt text of the logo.

Copy link
Copy Markdown
Member

@cscheid cscheid 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 have a couple of comments below:

  • we want a regression test here to ensure future changes catch our expectation for sidebar.logo-alt
  • I think we want to be conservative about newline changes in the ejs templates, and your commit added a newline at the end of a file that didn't have one

After those changes we should be good to merge!

Comment thread src/resources/projects/website/templates/sidebar.ejs
Comment thread src/resources/projects/website/templates/sidebar.ejs
Comment thread src/resources/projects/website/templates/sidebar.ejs
@cscheid cscheid added this to the v1.7 milestone Nov 18, 2024
@cscheid
Copy link
Copy Markdown
Member

cscheid commented Jan 31, 2025

Thanks for bringing this back! The changelog entry needs to be added to 1.7, not 1.6.

@mcanouil
Copy link
Copy Markdown
Collaborator Author

Indeed, now that 1.6 is out^^

@mcanouil
Copy link
Copy Markdown
Collaborator Author

Done

@cscheid cscheid merged commit 0070f6f into quarto-dev:main Mar 5, 2025
@mcanouil mcanouil deleted the fix/issue10251 branch March 5, 2025 13:12
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.

logo-alt broken when logo-href provided

2 participants