Skip to content

210 update galleries#265

Merged
dstopp merged 22 commits intomainfrom
210_update_galleries
Apr 7, 2026
Merged

210 update galleries#265
dstopp merged 22 commits intomainfrom
210_update_galleries

Conversation

@dstopp
Copy link
Copy Markdown
Collaborator

@dstopp dstopp commented Jan 27, 2026

  • Add template code to support individual gallery pages
  • Update the css for the gallery pages
  • Change the link for each thumbnail to forward to the associated photojournal page, https://science.nasa.gov/photojournal instead of the self-maintained PDS press release page. Note that the link is the original link w/the PIA number; the NASA photojournal is redirecting politely to the correct page. There is code to update the links but it is very timely and has a tendency to time out after an hour.
  • Added a 'back' button to return to the main gallery menu
  • For pages with more than 10 gallery images on a single page, added a menu at the bottom of the list of images for easier navigation
  • The 'first previous next last' nav modified to walk thru each of the available pages, including the expanded pages. Used to stop at the end of the expanded sections. Latest has been modified to always be the last item on the nav menu.
  • Standardized the landing page for each gallery to the first page in the menu (either by date or name of target) for consistency
  • Standardized the expanding menu options so they all expand the same way and same format for consistency and easy of update to the code.

Included are changes to accommodate the press release pages, but that part is not currently being implemented. That includes the press_release_data_table template, div_block.html template (which should be useful in other places) and sa few css changes. In addition, there were some minor scss changes required for framework upgrade.

These changes need to work in cooperation with the changes being implemented in rms_galleries. The individual gallery pages are still being generated by the rms_galleries python framework, but will now be generated as jekyll yaml templates.

Summary by CodeRabbit

  • New Features
    • New gallery navigation (pagination, "Jump to", persistent back link), redesigned image gallery with improved captions/previews, reusable markdown block/tag, and a press-release metadata table; updated gallery pages and titles; new gallery stylesheet.
  • Style
    • Enhanced gallery/navigation and press-release styling, table helpers, responsive search input, image-gallery layout and hover effects, and Sass color-handling updates.
  • Chores
    • Removed obsolete .gitignore entries.

dstopp added 5 commits March 11, 2025 12:30
…ncludes css changes

Changes to the gallery page also include standardizing the landing page to the first page in the menu (either by date or name of target)
Included are changes to accommodate the press release pages, but that is not currently being implemented.  That includes the press_release_data_table template, div_block.html template (which should be useful in other places) and sa few css changes.  In addition, there were some minor scss changes required for framework upgrade.
…pages. Includes css changes"

This reverts commit 92dfd93.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)

Walkthrough

Adds new Jekyll includes and a DivTag plugin; refactors gallery markup and gallery-related CSS; adds a press-release data table include and div_block include; updates galleries page links and asset references; and removes three ignore entries from website/.gitignore.

Changes

Cohort / File(s) Summary
Gitignore
website/.gitignore
Removed three ignored entries: assets.txt, external.txt, internal.txt.
Gallery includes & templates
website/_includes/gallery.html, website/_includes/image-gallery.html
Added gallery.html include (pagination, jump-to, back link) and image-gallery.html (replaced list markup with <gallery>/<gallery-object> and <figure>/<figcaption>, adjusted link and caption logic, removed inline list CSS).
Gallery CSS
website/assets/css/image-gallery.css, website/assets/css/style.css
Added image-gallery.css (flex layout, hover scaling, caption rules); updated style.css (gallery/jumpto styles, press-release/table helpers, searchbox positioning and minor layout tweaks).
Utility includes & plugin
website/_includes/div_block.html, website/_includes/press_release_data_table.html, website/_plugins/div_tag.rb
Added div_block.html include and div_block Liquid tag plugin (Jekyll::DivTag); added press_release_data_table.html to render a Cataloging Keywords table with conditional cells and link handling.
Sass changes
website/assets/css/main.scss
Imported sass:color and replaced lighten/darken with color.scale() for grey variants; removed some partial imports.
Head & header
website/_includes/head.html, website/_includes/header.html
Inserted stylesheet link for image-gallery.css in head; changed search input to width: 100% and removed specific padding rules for .gsib_a/.gsib_b.
Content updates
website/galleries.html
Updated page title to "NASA Press Release Image Galleries", switched image sources to site.assets_url, and updated many gallery links to year-specific target pages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title '210 update galleries' refers to a branch/ticket number and describes the general area of change but lacks specificity about the main modification. Consider a more descriptive title that highlights the primary change, such as 'Add gallery pagination and navigation templates' or 'Refactor gallery pages with updated navigation UI'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 12

🤖 Fix all issues with AI agents
In `@website/_includes/head.html`:
- Line 29: The stylesheet link with href "/assets/css/style.css" does not
include the Liquid prepend for site.baseurl like the other asset links; update
the href to use the same Liquid include pattern (prepend: site.baseurl) used on
the neighboring lines so the URL resolves under the configured baseurl for
subpath deployments.

In `@website/_includes/image-gallery.html`:
- Around line 30-44: The anchor tags that use target="_blank" to open links in a
new tab should also include rel="noopener noreferrer" to prevent reverse
tabnabbing security issues. Update the anchor tags in the image-gallery.html
section where target="_blank" is used, adding the attribute rel="noopener
noreferrer" alongside target="_blank".
- Around line 23-48: Replace the three occurrences of the nil-comparison check
"image.movie != nil" with a truthy check "image.movie": update the class
assignment block (where class is set to 'movie thumbnail'), the figcaption
inside the branch that uses image.href, and the figcaption inside the else
branch that builds the opus link; specifically change each "{% if image.movie !=
nil %}" to "{% if image.movie %}" so that false values are treated correctly as
falsy.

In `@website/_includes/press_release_data_table.html`:
- Around line 65-68: The anchor tag using target="_blank" for the Source link
does not include rel="noopener noreferrer" which is necessary for security.
Update the anchor tag within the press release row to add rel="noopener
noreferrer" alongside target="_blank" to prevent reverse-tabnabbing.

In `@website/_plugins/div_tag.rb`:
- Around line 8-11: Replace the Markdown conversion in the render method to use
the site’s Markdown converter instead of Jekyll::Renderer. Inside
render(context), obtain the site from context.registers[:site], get the
converter via site.find_converter_instance(Jekyll::Converters::Markdown), and
convert the super content with converter.convert(...). Then wrap the converted
content in the <div class="#{`@class`}">...</div> string as before. Remove the
Jekyll::Renderer.new(...).convert usage entirely.

In `@website/assets/css/style.css`:
- Line 294: Remove the leftover debug text "xyz" from the CSS comment "/*
homepage boxes xyz */" in style.css—either change it to a clean descriptive
comment like "/* homepage boxes */" or delete the comment entirely; locate the
comment text exactly as "/* homepage boxes xyz */" to make the edit.
- Around line 287-291: The rule for selector "div.press_release table" uses a
fixed width (width: 840px) which breaks on small screens; change it to use a
responsive approach by replacing the fixed width with a max-width (e.g.,
max-width: 840px) and allowing width: 100% or auto so the table scales down, and
optionally add a media query targeting small viewports to adjust
padding/font-size or stack content as needed; update the "div.press_release
table" rule and add the media query near related layout rules.
- Around line 784-789: The .subnav CSS rule is missing a trailing semicolon on
the background-color declaration; update the .subnav block (selector ".subnav")
to add the missing semicolon after "background-color: `#ddd`" so the declaration
becomes terminated properly and future properties can be added safely.

In `@website/galleries.html`:
- Around line 13-17: The galleries.html navigation contains many links (e.g.,
mercury_1996.html, venus.html, mars_1996.html, jupiter_1996.html,
target_phobos.html) that point to non-existent files and the deployment expects
a website_galleries/ directory from fabfile.py which is missing; fix this by
either (a) restoring/generating the missing gallery pages into
website/galleries/ (or the expected website_galleries/ output) using the gallery
generation script, ensuring filenames match the links in galleries.html, or (b)
if those pages were removed, update galleries.html to point to existing pages or
add redirects; also update fabfile.py to create/populate the website_galleries
output directory during build/deploy so the gallery pages are produced and
deployed.

In `@website/voyager/iss/inst_cat_na1.html`:
- Line 20: The markdown link on the line containing "[Instrument
Electronics](`#inst_elec`))" has an extra closing parenthesis; edit that link
(anchor "#inst_elec") to remove the redundant ')' so it becomes "[Instrument
Electronics](`#inst_elec`)" to restore correct markdown rendering.

In `@website/voyager/iss/inst_cat_na2.html`:
- Line 20: There is an extra closing parenthesis in the markdown link for
"Instrument Electronics" which breaks the link. Locate the entry containing
"Instrument Electronics" in the list and remove the duplicated closing
parenthesis so the link correctly uses only one closing parenthesis.

In `@website/voyager/iss/inst_cat_wa1.html`:
- Line 20: Replace the empty anchor href for the "Instrument Electronics" link
(currently href="#") with the correct section anchor "#inst_elec" so the link
navigates to the existing inst_elec anchor; update the <a> element for the
"Instrument Electronics" list item to use href="#inst_elec" (ensure the link
text "Instrument Electronics" and the target id "inst_elec" match exactly).

Comment thread website/_includes/head.html Outdated
Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/_includes/press_release_data_table.html
Comment thread website/_plugins/div_tag.rb
Comment thread website/assets/css/style.css Outdated
Comment thread website/galleries.html
Comment thread website/voyager/iss/inst_cat_na1.html Outdated
Comment thread website/voyager/iss/inst_cat_na2.html Outdated
Comment thread website/voyager/iss/inst_cat_wa1.html Outdated
… pages. Includes css changes"

This reverts commit 0925e3b.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 Fix all issues with AI agents
In `@website/_includes/gallery.html`:
- Around line 36-49: The conditional uses page.jump but the loop uses
page.jump_to, causing the block to be skipped; update the outer check to test
the actual collection (page.jump_to != nil and page.jump_to | length > 0) or
otherwise make the field names consistent (use page.jump everywhere or
page.jump_to everywhere) so the "Jump to" gallery renders when page.jump_to
contains items; adjust the conditional surrounding the div that currently
references page.jump to reference page.jump_to instead.

In `@website/_includes/image-gallery.html`:
- Around line 8-17: The CSS rule for the selector "gallery" is missing a
semicolon after the declaration align-items: flex-start which breaks parsing and
prevents subsequent properties from applying; update the gallery block (the
align-items declaration) to include the trailing semicolon so
margin-block-start, margin-block-end, margin-inline-start, margin-inline-end,
and padding-inline-start are parsed and applied correctly.
- Around line 20-36: The template uses nonstandard custom elements <gallery> and
<gallery-object> (symbols: gallery, gallery-object, image-gallery) which hurt
semantics/accessibility; replace them with semantic containers such as <div
role="group" class="image-gallery"> and <div class="gallery-object"> (or
<section> with appropriate ARIA roles/classes), update any CSS selectors from
gallery/gallery-object to .image-gallery/.gallery-object, ensure
figcaption/figure remain unchanged for semantics, and fix the closing container
indentation so the final closing tag matches the opening tag style.

In `@website/voyager/iss/inst_cat_na1.html`:
- Line 20: There is a typo in the markdown link for "Instrument Electronics"
where a double closing parenthesis is used. To fix it, remove the extra closing
parenthesis so that the link ends with a single closing parenthesis, correcting
the syntax for proper markdown linking.
- Line 128: Replace the typo in the HTML text fragment "remove radiometric an
geometric distortions from the flight" by changing "an" to "and" so the sentence
reads "remove radiometric and geometric distortions from the flight"; locate the
exact string in inst_cat_na1.html and update it accordingly.

In `@website/voyager/iss/inst_cat_na2.html`:
- Line 20: The markdown link for "Instrument Electronics" contains an extra
closing parenthesis — locate the line containing the list item "* [Instrument
Electronics](`#inst_elec`))" and remove the stray trailing ")" so the link reads
"* [Instrument Electronics](`#inst_elec`)"; ensure the anchor "#inst_elec" and
surrounding list formatting remain unchanged.

In `@website/voyager/iss/inst_cat_wa1.html`:
- Line 20: The "Instrument Electronics" link currently uses an empty anchor "#"
so it doesn't navigate; update the anchor in the nav/link element (the line
containing "Instrument Electronics") to point to the actual section id (for
example change href="#" to href="#instrument-electronics") and ensure the target
section element has a matching id attribute (id="instrument-electronics") so the
link correctly jumps to that section.
- Around line 304-313: The preformatted block for the "Instrument Filter ‘2 -
CLEAR’" entry duplicates the header text; remove the redundant line "Instrument
Filter '2 - CLEAR'" from inside the <pre><code> block so the block only contains
the filter metadata (Filter Name, Filter Type, Minimum/Maximum Wavelength,
Center Filter Wavelength), ensuring the visible header "**Instrument Filter ‘2 -
CLEAR’**" remains outside the code block and matches the format used by other
filter entries.

In `@website/voyager/iss/inst_cat_wa2.html`:
- Around line 167-168: Replace the typo substring "so the at the plaque" with
"so that the plaque" in the HTML content containing the sentence starting "light
transfer curve can be constructed by orienting the spacecraft..." (search for
the exact phrase "so the at the plaque" to locate the element) so the
calibration description reads correctly.
- Around line 298-300: Replace the raw HTML list item with the same markdown
list format used elsewhere in the file: change the HTML fragment containing the
anchor with text "Optics/filter response plots and data" pointing to
"inst_cal/vg2_wa_ch4js.html" into a markdown list entry (e.g., "* [Optics/filter
response plots and data](inst_cal/vg2_wa_ch4js.html)"). Ensure the link text and
URL remain identical to preserve behavior.

Comment thread website/_includes/gallery.html Outdated
Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/voyager/iss/inst_cat_na1.html Outdated
Comment thread website/voyager/iss/inst_cat_na1.html Outdated
Comment thread website/voyager/iss/inst_cat_na2.html Outdated
Comment thread website/voyager/iss/inst_cat_wa1.html Outdated
Comment thread website/voyager/iss/inst_cat_wa1.html Outdated
Comment thread website/voyager/iss/inst_cat_wa2.html Outdated
Comment thread website/voyager/iss/inst_cat_wa2.html Outdated
@dstopp dstopp requested a review from rfrenchseti January 27, 2026 21:38
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@website/_includes/image-gallery.html`:
- Around line 1-17: Remove the inline <style> block from the include and move
all CSS rules for the gallery into the shared stylesheet: copy the selectors and
rules for .image-gallery, .image-gallery gallery-object, .image-gallery
gallery-object a, .image-gallery gallery-object a figcaption, .image-gallery
gallery-object a img, .image-gallery gallery-object a figcaption img and the
gallery { ... } block into the central CSS file, preserving property
names/values and adjusting selector specificity if needed; then delete the
inline block from website/_includes/image-gallery.html so the include relies on
the shared stylesheet for gallery styling.

In `@website/_includes/press_release_data_table.html`:
- Around line 51-54: The first cell rendering the row label currently uses a
data cell (<td>{{ row.table_row }}</td>); change it to a row-header cell by
replacing that with a header cell element that includes scope="row" so assistive
tech recognizes it as the row header (locate the loop over page.catalog_table
and the expression row.table_row to update the element).

Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/_includes/press_release_data_table.html
Copy link
Copy Markdown
Collaborator

@rfrenchseti rfrenchseti left a comment

Choose a reason for hiding this comment

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

I'm not qualified to comment on this PR, except to note that it's not clear if the CodeRabbit issues have been addressed.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/style.css`:
- Around line 278-285: The a.press_release img rule is trying to center the
image with margin: auto but img is inline by default, so add display: block to
the selector to make margin:auto center it; update the a.press_release img CSS
(the rule block shown) to include display: block so float: none !important and
margin: auto will horizontally center the image and respect the
max-width/max-height constraints.
- Around line 278-285: The CSS rule for the selector "a.press_release img" uses
margin: auto to center but leaves the image inline, so add display: block to
that rule to make margin auto center the element; update the "a.press_release
img" declaration to include display: block alongside the existing float: none
!important, margin: auto, width/height, and max-size properties.
- Around line 791-793: The .subnav rule hides dropdowns by default and lacks a
complementary show rule and keyboard support; update the stylesheet so the
dropdown is shown on hover and when focused: remove or alter the existing
`.subnav:not(:hover) { display: none; }` behavior by adding a CSS show rule such
as targeting `.nav-item:hover .subnav { display: block; }` and include keyboard
accessibility via `.subnav:focus-within` or `.nav-item:focus-within .subnav` so
the subnav remains visible when any descendant receives keyboard focus (ensure
these selectors match the existing `.nav-item` and `.subnav` class names used in
the markup).
- Around line 791-793: The CSS currently hides dropdowns with
".subnav:not(:hover) { display: none; }", which prevents keyboard users from
accessing subnavs; update the selector to also allow focus by changing it to
exclude when the subnav or its descendants have focus (i.e., add :focus-within)
and ensure there is a corresponding show rule for hover on the parent (e.g., add
a rule that targets ".nav-item:hover .subnav" or ".subnav-toggle:focus ~
.subnav" to set display: block) so both keyboard focus and CSS hover paths
reveal the subnav; modify selectors referencing .subnav, .nav-item, and
.subnav-toggle accordingly.

Comment thread website/assets/css/style.css
Comment thread website/assets/css/style.css Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
website/assets/css/style.css (2)

791-793: ⚠️ Potential issue | 🟠 Major

Subnav keyboard accessibility still unaddressed — :focus-within exclusion missing.

This was raised in a previous review and remains unresolved. Two problems persist:

  1. No CSS show rule exists (no .nav-item:hover .subnav { display: block; } or equivalent), so the CSS-only hover path is dead and the subnav is only revealable via JavaScript.
  2. When a keyboard user tabs into a subnav descendant, :hover is inactive, so :not(:hover) fires and hides it.
♿ Proposed fix
-.subnav:not(:hover) {
+.subnav:not(:hover):not(:focus-within) {
     display: none;
 }

If a pure-CSS hover path alongside the JS toggle is desired, also add:

.nav-item:hover .subnav {
    display: block;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/style.css` around lines 791 - 793, The .subnav hide rule
using .subnav:not(:hover) improperly hides the submenu for keyboard users and
there's no matching show rule; update the CSS so keyboard focus keeps the subnav
visible by changing the hide selector to exclude focus (use
:not(:hover):not(:focus-within)) and add a complementary show rule such as
targeting .nav-item:hover .subnav to display the submenu for mouse users,
ensuring both .subnav and .nav-item selectors are updated so the JS toggle
remains compatible.

278-285: ⚠️ Potential issue | 🟡 Minor

margin: auto still won't center a.press_release imgdisplay: block is missing.

This was raised in a previous review and remains unaddressed. Without display: block, img stays inline and margin: auto has no centering effect.

🐛 Proposed fix
 a.press_release img {
     float: none !important;
+    display: block;
     margin: auto;
     width: auto;
     height: auto;
     max-width: 1200px;
     max-height: 800px;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/style.css` around lines 278 - 285, The selector
a.press_release img currently uses margin: auto but lacks display:block so
horizontal centering won't work; update the a.press_release img rule to include
display: block (e.g., add display: block; to the existing declarations) so
margin: auto can center the image as intended while keeping the existing
max-width/max-height settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/style.css`:
- Around line 805-807: The hover underline on .jumpto.nav_container a:hover is
being suppressed by the global rule nav a { text-decoration: none !important };
remove the !important from that global rule (nav a) and make it a normal
declaration, then restore/explicitly set the intended behavior for the general
nav links if needed (e.g., keep nav a { text-decoration: none }), letting the
existing .jumpto.nav_container a:hover rule take effect; alternatively, if
removing !important is unacceptable, add a more specific override such as
.jumpto.nav_container a:hover { text-decoration: underline !important; } so the
underline wins — update either the nav a or .jumpto.nav_container a:hover rule
accordingly.

---

Duplicate comments:
In `@website/assets/css/style.css`:
- Around line 791-793: The .subnav hide rule using .subnav:not(:hover)
improperly hides the submenu for keyboard users and there's no matching show
rule; update the CSS so keyboard focus keeps the subnav visible by changing the
hide selector to exclude focus (use :not(:hover):not(:focus-within)) and add a
complementary show rule such as targeting .nav-item:hover .subnav to display the
submenu for mouse users, ensuring both .subnav and .nav-item selectors are
updated so the JS toggle remains compatible.
- Around line 278-285: The selector a.press_release img currently uses margin:
auto but lacks display:block so horizontal centering won't work; update the
a.press_release img rule to include display: block (e.g., add display: block; to
the existing declarations) so margin: auto can center the image as intended
while keeping the existing max-width/max-height settings.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 669da67 and 4cf8e0f.

📒 Files selected for processing (1)
  • website/assets/css/style.css

Comment thread website/assets/css/style.css Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
website/_includes/image-gallery.html (1)

1-17: 🧹 Nitpick | 🔵 Trivial

Move inline gallery CSS into the shared stylesheet.
Keeping these rules inline duplicates CSS across pages and bypasses caching; please relocate them into website/assets/css/style.css and remove the inline block.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_includes/image-gallery.html` around lines 1 - 17, Move the inline
CSS block out of the template and into the shared stylesheet: copy the rules for
.image-gallery, .image-gallery gallery-object, .image-gallery gallery-object a,
.image-gallery gallery-object a figcaption, .image-gallery gallery-object a img,
.image-gallery gallery-object a figcaption img, and gallery selector into the
global style sheet (style.css), keeping selectors and declarations intact
(adjust selectors to use correct class/element names if necessary), then remove
the entire <style>…</style> block from website/_includes/image-gallery.html so
the template no longer contains duplicated inline CSS and relies on the shared
stylesheet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@website/_includes/image-gallery.html`:
- Around line 1-17: Move the inline CSS block out of the template and into the
shared stylesheet: copy the rules for .image-gallery, .image-gallery
gallery-object, .image-gallery gallery-object a, .image-gallery gallery-object a
figcaption, .image-gallery gallery-object a img, .image-gallery gallery-object a
figcaption img, and gallery selector into the global style sheet (style.css),
keeping selectors and declarations intact (adjust selectors to use correct
class/element names if necessary), then remove the entire <style>…</style> block
from website/_includes/image-gallery.html so the template no longer contains
duplicated inline CSS and relies on the shared stylesheet.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cf8e0f and 4ad2a90.

📒 Files selected for processing (2)
  • website/_includes/image-gallery.html
  • website/assets/css/style.css

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (2)
website/_plugins/div_tag.rb (1)

9-13: ⚠️ Potential issue | 🔴 Critical

render currently has a runtime break and unsafe interpolation.

At Line 12, converter is used before assignment (Line 11 assignment is missing), and at Line 13 #{@Class} executes a shell command instead of using @class. This will either fail at runtime or produce unsafe behavior.

🔧 Proposed fix
     def render(context)
       site = context.registers[:site]
       content = super
-      rendered = converter.convert(content)
-      "<div class=\"#{`@class`}\">#{rendered}</div>"    end
+      converter = site.find_converter_instance(Jekyll::Converters::Markdown)
+      rendered = converter.convert(content)
+      "<div class=\"#{`@class`}\">#{rendered}</div>"
+    end
#!/bin/bash
set -euo pipefail

file="$(fd -t f div_tag.rb | head -n1)"
echo "Inspecting: $file"
nl -ba "$file" | sed -n '1,120p'

echo
echo "Check for converter assignment:"
rg -nP '\bconverter\s*=' "$file" -C2 || true

echo
echo "Check for shell-executing interpolation pattern:"
rg -n '\#\{`@class`\}' "$file" -C2 || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_plugins/div_tag.rb` around lines 9 - 13, The render method in
div_tag.rb uses converter before it's assigned and uses unsafe shell-executing
interpolation #{`@class`}; fix by assigning converter =
site.find_converter_instance(Jekyll::Converters::Markdown) after obtaining site
(context.registers[:site]) and replace the backtick interpolation with safe Ruby
instance variable interpolation #{`@class`} (or better, use a safer accessor or
escape HTML if `@class` can contain user input) in the return string so render
returns "<div class=\"#{`@class`}\">#{rendered}</div>" after calling rendered =
converter.convert(content).
website/_includes/image-gallery.html (1)

1-18: 🛠️ Refactor suggestion | 🟠 Major

Remove duplicate inline styles now that image-gallery.css exists.

These inline styles are now duplicated in website/assets/css/image-gallery.css, which is loaded via head.html. The duplication:

  • Increases page weight on every gallery page
  • Creates maintenance burden (changes must be made in two places)
  • Bypasses browser caching benefits of external stylesheets

Since the external stylesheet is now loaded globally, this entire <style> block can be removed.

♻️ Proposed fix: Remove inline styles
-<style>
-    .image-gallery {overflow: hidden; padding-inline-start: 0;text-wrap:nowrap;}
-    .image-gallery gallery-object {text-align: center; margin: 0.2%; width: 12.5em; min-height: 150px;}
-    .image-gallery gallery-object a {text-decoration: none!important; color: `#111`;}
-    .image-gallery gallery-object a figcaption {font-size: 10pt; overflow: hidden; padding: 3px 0; text-align: center; text-wrap: auto}
-    .image-gallery gallery-object a img {margin-left: auto; margin-right: auto; padding-top: .75em; text-wrap: wrap; font-size: 75%;}
-    .image-gallery gallery-object a figcaption img {padding-top: 0; padding-bottom: 3px;}
-
-    gallery {
-        display: flex;
-        flex-wrap: wrap;
-        margin-block-start: 1em;
-        margin-block-end: 1em;
-        margin-inline-start: 0px;
-        margin-inline-end: 0px;
-        padding-inline-start: 40px;
-    }
-</style>
-
 <gallery class="image-gallery">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_includes/image-gallery.html` around lines 1 - 18, Remove the
duplicated inline <style> block in website/_includes/image-gallery.html (the
block that defines .image-gallery, gallery, and gallery-object rules) since
these styles are now provided by the external image-gallery.css loaded from
head.html; simply delete the entire <style>...</style> section so the page uses
the cached external stylesheet (verify no other inline rules remain in that
file).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/_includes/image-gallery.html`:
- Around line 29-35: Replace the explicit `!= nil` comparisons with truthy
checks for the image properties so behavior matches other checks (e.g., use `if
image.href` and `if image.src` instead of `if image.href != nil` / `if image.src
!= nil`); update the conditional logic around `image.href` and `image.src` in
the image rendering template to mirror the existing `if image.movie` style so
falsey values are handled consistently.

In `@website/assets/css/image-gallery.css`:
- Around line 1-5: The .image-gallery rules are duplicated inline and in the
external stylesheet; remove the inline <style> block containing the
.image-gallery rules from the image-gallery include so the site uses the single
source in image-gallery.css (ensure head.html continues to load
image-gallery.css and that no other inline rules for .image-gallery remain);
leave the selector and rules only in image-gallery.css to avoid duplication and
maintenance drift.
- Around line 27-33: Remove the ineffective text-wrap declaration from the
.image-gallery gallery-object a img rule since text-wrap does not apply to img
(a replaced element); locate the CSS selector ".image-gallery gallery-object a
img" and delete the "text-wrap: wrap;" line, and optionally remove or comment on
"font-size: 75%" if you don't intend to style alt text fallback, leaving only
layout-relevant rules (margin/padding) in that selector.
- Around line 24-25: Add the missing semicolon to the CSS declaration so the
rule "text-wrap: auto" in website/assets/css/image-gallery.css becomes
"text-wrap: auto;"—update the declaration inside the same selector block that
contains this line to ensure proper termination and avoid issues when adding
subsequent properties.
- Around line 40-48: The CSS uses an unknown type selector "gallery" which
linters flag; replace that rule block by moving its declarations into the
existing class selector ".image-gallery" (or merge them into .image-gallery's
rule) so the flex, flex-wrap, margin-*, and padding-inline-start styles apply to
.image-gallery instead of the "gallery" element; alternatively, if you
intentionally want a custom element, update the Stylelint config
(selector-type-no-unknown) or add an inline suppression comment, but prefer
updating the selector to ".image-gallery" referenced in the file.

---

Duplicate comments:
In `@website/_includes/image-gallery.html`:
- Around line 1-18: Remove the duplicated inline <style> block in
website/_includes/image-gallery.html (the block that defines .image-gallery,
gallery, and gallery-object rules) since these styles are now provided by the
external image-gallery.css loaded from head.html; simply delete the entire
<style>...</style> section so the page uses the cached external stylesheet
(verify no other inline rules remain in that file).

In `@website/_plugins/div_tag.rb`:
- Around line 9-13: The render method in div_tag.rb uses converter before it's
assigned and uses unsafe shell-executing interpolation #{`@class`}; fix by
assigning converter = site.find_converter_instance(Jekyll::Converters::Markdown)
after obtaining site (context.registers[:site]) and replace the backtick
interpolation with safe Ruby instance variable interpolation #{`@class`} (or
better, use a safer accessor or escape HTML if `@class` can contain user input) in
the return string so render returns "<div class=\"#{`@class`}\">#{rendered}</div>"
after calling rendered = converter.convert(content).

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cf8e0f and 01e3a20.

📒 Files selected for processing (5)
  • website/_includes/head.html
  • website/_includes/image-gallery.html
  • website/_plugins/div_tag.rb
  • website/assets/css/image-gallery.css
  • website/assets/css/style.css

Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/assets/css/image-gallery.css Outdated
Comment thread website/assets/css/image-gallery.css
Comment thread website/assets/css/image-gallery.css Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
website/_includes/image-gallery.html (1)

10-13: ⚠️ Potential issue | 🟡 Minor

Use truthy checks for image.href and image.src (not != nil).

Line 10 and Line 12 still use != nil; in Liquid this can treat explicit false as present and choose the wrong branch.

♻️ Proposed fix
-            {% if image.href != nil %}
+            {% if image.href %}
                 <a href="{{ image.href }}" target="_blank" rel="noopener noreferrer">
-                {% if image.src != nil %}
+                {% if image.src %}
                     <img src="{{ site.assets_url }}{{ image.src }}" alt="{{ image.alt }}" />
                 {% else %}
                     <img src="{{ image.external_src }}" alt="{{ image.alt }}" />
                 {% endif %}
#!/bin/bash
# Verify remaining nil-comparison checks in the include
rg -n '\{%\s*if\s+image\.(href|src)\s*!=\s*nil\s*%\}' website/_includes/image-gallery.html

# Verify whether any data files explicitly set href/src to false (risk case)
rg -nP '^\s*(href|src):\s*false\s*$' -g '*.yml' -g '*.yaml' -g '*.json'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_includes/image-gallery.html` around lines 10 - 13, Replace the
explicit nil comparisons with truthy checks in the image-gallery include: update
the conditional that checks image.href (currently "{% if image.href != nil %}")
to use a truthy test ("{% if image.href %}") and do the same for image.src
(currently "{% if image.src != nil %}") so false values won't be treated as
present; keep the existing anchor/img structure and attributes (the blocks
around image.href and image.src) intact while making these substitutions.
website/assets/css/image-gallery.css (1)

40-48: ⚠️ Potential issue | 🟠 Major

gallery selector is currently failing configured linters.

Line 40 uses an unknown type selector, and static analysis already reports this as an error. Move these declarations to a class-based selector (e.g., .image-gallery) or explicitly allow this selector in lint config.

♻️ Proposed fix (class-based)
 .image-gallery {
     overflow: hidden;
     padding-inline-start: 0;
     text-wrap: nowrap;
+    display: flex;
+    flex-wrap: wrap;
+    margin-block-start: 1em;
+    margin-block-end: 1em;
+    margin-inline-start: 0px;
+    margin-inline-end: 0px;
+    padding-inline-start: 40px;
 }
@@
-gallery {
-    display: flex;
-    flex-wrap: wrap;
-    margin-block-start: 1em;
-    margin-block-end: 1em;
-    margin-inline-start: 0px;
-    margin-inline-end: 0px;
-    padding-inline-start: 40px;
-}
#!/bin/bash
# Confirm selector presence
rg -n '^\s*gallery\s*\{' website/assets/css/image-gallery.css

# Confirm linter rules that reject unknown type selectors
rg -n 'selector-type-no-unknown|noUnknownTypeSelector|ignoreTypes' -g '*stylelint*' -g '*biome*' -g 'package.json' -g '*.json' -g '*.yaml' -g '*.yml'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/image-gallery.css` around lines 40 - 48, The CSS uses an
unknown type selector "gallery" which fails linters; rename the selector to a
class-based selector (e.g., change the rule for gallery to .image-gallery) and
move the same declarations under .image-gallery in
website/assets/css/image-gallery.css, then update any HTML that uses <gallery>
to use class="image-gallery" (or adjust templates/components that reference the
gallery element) so linters accept it and styles still apply.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/_plugins/div_tag.rb`:
- Around line 8-14: The render method contains leftover diff markers and
malformed statements; remove the `+`/`-` characters, ensure you retrieve site
from context.registers[:site], call
site.find_converter_instance(Jekyll::Converters::Markdown) to assign to
converter, call converter.convert(content) to get rendered, interpolate the
instance variable via "#{`@class`}" (not backticks), and return "<div
class=\"#{`@class`}\">#{rendered}</div>" with the final end token on its own line;
update references to the render, site, converter, find_converter_instance,
convert, and `@class` symbols accordingly.

---

Duplicate comments:
In `@website/_includes/image-gallery.html`:
- Around line 10-13: Replace the explicit nil comparisons with truthy checks in
the image-gallery include: update the conditional that checks image.href
(currently "{% if image.href != nil %}") to use a truthy test ("{% if image.href
%}") and do the same for image.src (currently "{% if image.src != nil %}") so
false values won't be treated as present; keep the existing anchor/img structure
and attributes (the blocks around image.href and image.src) intact while making
these substitutions.

In `@website/assets/css/image-gallery.css`:
- Around line 40-48: The CSS uses an unknown type selector "gallery" which fails
linters; rename the selector to a class-based selector (e.g., change the rule
for gallery to .image-gallery) and move the same declarations under
.image-gallery in website/assets/css/image-gallery.css, then update any HTML
that uses <gallery> to use class="image-gallery" (or adjust templates/components
that reference the gallery element) so linters accept it and styles still apply.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cf8e0f and 3430a72.

📒 Files selected for processing (5)
  • website/_includes/head.html
  • website/_includes/image-gallery.html
  • website/_plugins/div_tag.rb
  • website/assets/css/image-gallery.css
  • website/assets/css/style.css

Comment thread website/_plugins/div_tag.rb Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
website/_plugins/div_tag.rb (1)

8-17: ⚠️ Potential issue | 🔴 Critical

Critical: Fatal syntax errors prevent this plugin from loading.

This file contains multiple syntax errors that will cause Ruby to fail at parse time:

  1. Diff markers committed as code (lines 9, 11-14): The -+, --, and + characters at line starts are invalid Ruby syntax—these appear to be merge conflict artifacts or improperly applied diff output.

  2. Shell command execution via backticks (line 13): `@class` executes @class as a shell command. Use #{@Class} for string interpolation.

  3. Missing module end (line 15): The end closes the DivTag class, but the Jekyll module opened on line 1 is never closed.

RuboCop confirms: unexpected token kEND at line 14 and unexpected token $end at line 17.

,

🐛 Proposed fix
 module Jekyll
   class DivTag < Liquid::Block
     def initialize(tag_name, markup, tokens)
       super
       `@class` = markup.strip
     end
 
     def render(context)
--+    site = context.registers[:site]
+      site = context.registers[:site]
       content = super
---    converter = site.find_converter_instance(Jekyll::Converters::Markdown)
-+     rendered = converter.convert(content)
-+     "<div class=\"#{`@class`}\">#{rendered}</div>"
-+   end
-end
+      converter = site.find_converter_instance(Jekyll::Converters::Markdown)
+      rendered = converter.convert(content)
+      "<div class=\"#{`@class`}\">#{rendered}</div>"
+    end
+  end
+end
 
 Liquid::Template.register_tag('div_block', Jekyll::DivTag)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_plugins/div_tag.rb` around lines 8 - 17, Remove the stray diff
markers and fix syntax in the Jekyll::DivTag plugin: delete leading diff
artifacts (-+, --, +) so the code is valid Ruby, ensure you retrieve site via
context.registers[:site] inside render, get the Markdown converter with
site.find_converter_instance(Jekyll::Converters::Markdown), change shell
backticks `` `@class` `` to Ruby interpolation #{`@class`} when building the div
string in render, and add the missing end to close the surrounding module Jekyll
so the class Jekyll::DivTag and Liquid::Template.register_tag('div_block',
Jekyll::DivTag) are inside a properly closed module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/image-gallery.css`:
- Around line 1-12: Remove the duplicate padding-inline-start declaration in the
.image-gallery rule: keep only the intended value (40px) and delete the earlier
padding-inline-start: 0; so the stylesheet has a single, clear
padding-inline-start for the .image-gallery selector.

---

Duplicate comments:
In `@website/_plugins/div_tag.rb`:
- Around line 8-17: Remove the stray diff markers and fix syntax in the
Jekyll::DivTag plugin: delete leading diff artifacts (-+, --, +) so the code is
valid Ruby, ensure you retrieve site via context.registers[:site] inside render,
get the Markdown converter with
site.find_converter_instance(Jekyll::Converters::Markdown), change shell
backticks `` `@class` `` to Ruby interpolation #{`@class`} when building the div
string in render, and add the missing end to close the surrounding module Jekyll
so the class Jekyll::DivTag and Liquid::Template.register_tag('div_block',
Jekyll::DivTag) are inside a properly closed module.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cf8e0f and 60e6d45.

📒 Files selected for processing (5)
  • website/_includes/head.html
  • website/_includes/image-gallery.html
  • website/_plugins/div_tag.rb
  • website/assets/css/image-gallery.css
  • website/assets/css/style.css

Comment thread website/assets/css/image-gallery.css
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
website/_plugins/div_tag.rb (1)

8-13: ⚠️ Potential issue | 🔴 Critical

Fix the malformed render method before this plugin can load.

The unary +/- markers on Lines 9-13 are still committed as Ruby code, and #{\@class`}` on Line 13 runs a shell command instead of interpolating the instance variable. That leaves the plugin broken before any page can render.

🐛 Proposed fix
     def render(context)
-+      site = context.registers[:site]
+      site = context.registers[:site]
       content = super
--      converter = site.find_converter_instance(Jekyll::Converters::Markdown)
-+      rendered = converter.convert(content)
-+      "<div class=\"#{`@class`}\">#{rendered}</div>"    end
+      converter = site.find_converter_instance(Jekyll::Converters::Markdown)
+      rendered = converter.convert(content)
+      "<div class=\"#{`@class`}\">#{rendered}</div>"
     end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_plugins/div_tag.rb` around lines 8 - 13, The render method contains
leftover diff markers and incorrect interpolation; remove the stray '+'/'-'
characters, obtain site from context.registers[:site], get the Markdown
converter with site.find_converter_instance(Jekyll::Converters::Markdown) into a
local variable (e.g., converter), call converter.convert(content) to produce
rendered, and return the wrapper string using Ruby string interpolation of the
instance variable (use #{`@class`} not backticks) so render(context) returns "<div
class=\"#{`@class`}\">#{rendered}</div>".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/_includes/image-gallery.html`:
- Around line 10-17: The current change makes thumbnails use {{ site.assets_url
}}{{ image.src }} and replaces captions with image.title when image.href is set,
which breaks preview-relative src paths and blanks captions; update the
anchor-handling block so img src selection matches the original non-link logic:
use image.external_src if present, otherwise use image.src without forcing the
site.assets_url prefix for preview-relative paths (e.g., detect if image.src is
already a relative/preview path like "holdings/..." and leave it as-is), and
preserve the caption by rendering image.title only if set else falling back to
image.filename (i.e., use image.title || image.filename) so link-only updates do
not break thumbnails or erase captions; touch the conditional logic around
image.href, image.src, image.external_src, image.title and image.filename to
implement this fallback behavior.
- Around line 25-31: The figcaption is currently outside the figure element,
making it a sibling and invalid HTML; move the <figcaption> block so it is the
last child inside the <figure> element (i.e., place the figcaption markup
immediately before the closing </figure>), keeping the conditional image check
for image.movie and the {{ title }} content intact so styling continues to apply
correctly.

---

Duplicate comments:
In `@website/_plugins/div_tag.rb`:
- Around line 8-13: The render method contains leftover diff markers and
incorrect interpolation; remove the stray '+'/'-' characters, obtain site from
context.registers[:site], get the Markdown converter with
site.find_converter_instance(Jekyll::Converters::Markdown) into a local variable
(e.g., converter), call converter.convert(content) to produce rendered, and
return the wrapper string using Ruby string interpolation of the instance
variable (use #{`@class`} not backticks) so render(context) returns "<div
class=\"#{`@class`}\">#{rendered}</div>".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1146980e-aa9e-4dc3-8d74-c19b493a264a

📥 Commits

Reviewing files that changed from the base of the PR and between 4cf8e0f and 1304b40.

📒 Files selected for processing (6)
  • website/_includes/gallery.html
  • website/_includes/head.html
  • website/_includes/image-gallery.html
  • website/_plugins/div_tag.rb
  • website/assets/css/image-gallery.css
  • website/assets/css/style.css

Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/_includes/image-gallery.html Outdated
Copy link
Copy Markdown
Collaborator

@matthewtiscareno matthewtiscareno left a comment

Choose a reason for hiding this comment

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

@dstopp and I looked though the new page versions together, and everything looks really nice.

All the index pages have been redone, but the individual photo landing pages are unchanged. This will remain the case until we hear back from Photojournal about whether they are fixing their pages sufficiently for us to support them more directly.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/_includes/image-gallery.html`:
- Around line 14-16: The fallback referencing image.external_src in the
image-gallery.html template is dead code; remove the else branch that outputs
<img src="{{ image.external_src }}"> (and its closing {% endif %} if you
collapse the conditional) so the template only uses the validated image.src path
(keep the existing {% if image.src %} guard), or alternatively replace the else
block with a single-line comment noting external_src is intentionally
unused/reserved for future use; target the image.external_src reference and the
surrounding else block in the image-gallery.html template.

In `@website/assets/css/image-gallery.css`:
- Around line 18-21: The rule using "!important" on the selector ".image-gallery
gallery-object a" should be removed; replace it by increasing selector
specificity or fixing the selector to target the element correctly (e.g., use
the proper descendant selector like ".image-gallery .gallery-object a" or a more
specific class/id) so normal cascade overrides apply, and drop "!important" from
the text-decoration declaration in the CSS for the gallery link styling.
- Around line 23-29: There are two conflicting figcaption rules (.image-gallery
gallery-object a figcaption and .image-gallery gallery-object figcaption);
consolidate them by removing the duplicate and using a single selector (e.g.,
.image-gallery gallery-object figcaption) so all figcaptions share the same
styles regardless of being wrapped in an anchor, or if the anchor-specific style
is intentional, document the distinction and make the selector specificity
explicit; update/remove the rule for .image-gallery gallery-object a figcaption
and keep the unified rule in image-gallery.css to avoid inconsistent styling.
- Around line 54-56: The global figcaption rule is too broad and may affect
figcaption elements outside the gallery; scope it to the gallery selector (e.g.,
.image-gallery figcaption or .gallery__item figcaption) by replacing the bare
figcaption rule with a scoped selector used by the image gallery component and
ensure you keep the intended wrapping behavior (use the gallery's class name
present in this CSS or markup to locate the correct selector).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 652ea0f9-e2f7-4767-bb3d-84f870095810

📥 Commits

Reviewing files that changed from the base of the PR and between 1304b40 and 79fca74.

📒 Files selected for processing (2)
  • website/_includes/image-gallery.html
  • website/assets/css/image-gallery.css

Comment thread website/_includes/image-gallery.html Outdated
Comment thread website/assets/css/image-gallery.css
Comment thread website/assets/css/image-gallery.css
Comment thread website/assets/css/image-gallery.css Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
website/_includes/image-gallery.html (1)

10-17: ⚠️ Potential issue | 🟠 Major

image.href branch still uses a different thumbnail path and can regress previews.

Line 12 builds {{ site.assets_url }}{{ image.src }}, while Line 16 uses the preview path prefix. If image.src is still filename-style in table data, href-enabled items will resolve to broken thumbnail URLs.

Suggested fix (normalize thumbnail source resolution)
             {% if image.href %}
                 <a href="{{ image.href }}" target="_blank" rel="noopener noreferrer">
-                    <img src="{{ site.assets_url }}{{ image.src }}" alt="{{ image.alt }}" />
+                    <img src="{{ site.assets_url }}holdings/previews/{{ image.src }}" alt="{{ image.alt }}" />
                 </a>
             {% else %}
                 <a href="{{ site.opus_url }}{{ page.opus_share }}{{ image.filename | downcase }}" target="_blank" rel="noopener noreferrer">
                     <img src="{{ site.assets_url }}holdings/previews/{{ image.src }}" alt="{{ image.alt }}" />
                 </a>
             {% endif %}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/_includes/image-gallery.html` around lines 10 - 17, The href branch
still constructs the thumbnail src as "{{ site.assets_url }}{{ image.src }}"
which differs from the else branch's preview path and can produce broken
thumbnails; update the branch that checks image.href (in the image-gallery
template where image.href and image.src are used) to use the same preview prefix
(holdings/previews) with site.assets_url so both branches normalize thumbnail
source resolution and point to the same holdings/previews/{{ image.src }}
location.
website/assets/css/image-gallery.css (1)

23-29: ⚠️ Potential issue | 🟡 Minor

a figcaption selector does not match current markup.

In website/_includes/image-gallery.html, <figcaption> is outside <a>, so Lines 23-29 never apply. Move these caption styles onto the non-anchor selector and keep one consolidated figcaption rule.

Suggested consolidation
-.image-gallery gallery-object a figcaption {
+.image-gallery gallery-object figcaption {
     font-size: 10pt;
     overflow: hidden;
-    padding: 3px 0;
+    padding: 3px 2px 2ex 2px;
+    padding-top: 1ex;
     text-align: center;
     text-wrap: auto;
 }
@@
-.image-gallery gallery-object figcaption {
-    padding-top: 1ex;
-    padding-bottom: 2ex;
-    padding-left: 2px;
-    padding-right: 2px;
-}

Also applies to: 42-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/image-gallery.css` around lines 23 - 29, The CSS rule
".image-gallery gallery-object a figcaption" doesn’t match markup because
<figcaption> sits outside the <a>; move the caption styles (font-size, overflow,
padding, text-align, text-wrap) from the "a figcaption" rule onto the non-anchor
selector (e.g. ".image-gallery gallery-object figcaption") and remove/merge the
duplicate rule at lines 42-47 so there is a single consolidated ".image-gallery
gallery-object figcaption" rule containing those properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/image-gallery.css`:
- Around line 49-52: The selector figure.thumbnail is global and should be
scoped to the gallery component; update the CSS rule so it targets
.image-gallery figure.thumbnail instead of figure.thumbnail (preserve the
existing properties border: none; and max-height: 100px;) so the styles are
component-local and won't affect other pages.

---

Duplicate comments:
In `@website/_includes/image-gallery.html`:
- Around line 10-17: The href branch still constructs the thumbnail src as "{{
site.assets_url }}{{ image.src }}" which differs from the else branch's preview
path and can produce broken thumbnails; update the branch that checks image.href
(in the image-gallery template where image.href and image.src are used) to use
the same preview prefix (holdings/previews) with site.assets_url so both
branches normalize thumbnail source resolution and point to the same
holdings/previews/{{ image.src }} location.

In `@website/assets/css/image-gallery.css`:
- Around line 23-29: The CSS rule ".image-gallery gallery-object a figcaption"
doesn’t match markup because <figcaption> sits outside the <a>; move the caption
styles (font-size, overflow, padding, text-align, text-wrap) from the "a
figcaption" rule onto the non-anchor selector (e.g. ".image-gallery
gallery-object figcaption") and remove/merge the duplicate rule at lines 42-47
so there is a single consolidated ".image-gallery gallery-object figcaption"
rule containing those properties.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5f2efd47-5d21-499e-8e9d-5b7dc75e34a9

📥 Commits

Reviewing files that changed from the base of the PR and between 79fca74 and d6dcbbc.

📒 Files selected for processing (2)
  • website/_includes/image-gallery.html
  • website/assets/css/image-gallery.css

Comment thread website/assets/css/image-gallery.css Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
website/assets/css/image-gallery.css (1)

23-29: ⚠️ Potential issue | 🟠 Major

figcaption selector does not match the current gallery markup.

website/_includes/image-gallery.html places <figcaption> outside <a>, so .image-gallery gallery-object a figcaption never applies. Caption font/overflow/wrap styles are effectively dropped.

🔧 Proposed fix
-.image-gallery gallery-object a figcaption {
+.image-gallery gallery-object figcaption {
     font-size: 10pt;
     overflow: hidden;
-    padding: 3px 0;
+    padding: 3px 2px 2ex;
     text-align: center;
     text-wrap: auto;
 }
-
-.image-gallery gallery-object figcaption {
-    padding-top: 1ex;
-    padding-bottom: 2ex;
-    padding-left: 2px;
-    padding-right: 2px;
-}

Also applies to: 42-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/image-gallery.css` around lines 23 - 29, The CSS selector
".image-gallery gallery-object a figcaption" doesn't match markup because
figcaption is outside the anchor; update the selector(s) to target the actual
element (e.g., select figcaption under the gallery object) so caption styles
apply — replace the incorrect selector with one that targets ".image-gallery
.gallery-object figcaption" (and similarly adjust the duplicate selectors on
lines ~42-47) to restore font-size, overflow, padding, text-align and wrapping
rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/image-gallery.css`:
- Around line 62-65: The .gallery.page_menu rule uses a fixed padding-right:
20em which breaks centering on small viewports; replace the hardcoded offset
with a layout-based centering approach (e.g., make the container a flex/grid and
use justify-content: center or place-items: center, or use margin: 0 auto on a
constrained width element) and remove the padding-right declaration so the page
menu centers responsively.

---

Duplicate comments:
In `@website/assets/css/image-gallery.css`:
- Around line 23-29: The CSS selector ".image-gallery gallery-object a
figcaption" doesn't match markup because figcaption is outside the anchor;
update the selector(s) to target the actual element (e.g., select figcaption
under the gallery object) so caption styles apply — replace the incorrect
selector with one that targets ".image-gallery .gallery-object figcaption" (and
similarly adjust the duplicate selectors on lines ~42-47) to restore font-size,
overflow, padding, text-align and wrapping rules.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8df216ee-8be1-472a-8a7f-f5c225c34b6f

📥 Commits

Reviewing files that changed from the base of the PR and between d6dcbbc and 87e26c8.

📒 Files selected for processing (1)
  • website/assets/css/image-gallery.css

Comment thread website/assets/css/image-gallery.css Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
website/assets/css/image-gallery.css (1)

23-29: ⚠️ Potential issue | 🟠 Major

Figcaption typography rule is unreachable with current DOM.

figcaption is a sibling of <a>, not a descendant, so the selector on Line 23 never matches. Move those declarations to the existing gallery-object figcaption rule to ensure caption styling is applied.

♻️ Proposed fix
-.image-gallery gallery-object a figcaption {
-    font-size: 10pt;
-    overflow: hidden;
-    padding: 3px 0;
-    text-align: center;
-    text-wrap: auto;
-}
-
 .image-gallery gallery-object figcaption {
+    font-size: 10pt;
+    overflow: hidden;
+    text-align: center;
+    text-wrap: auto;
+    padding: 3px 0;
     padding-top: 1ex;
     padding-bottom: 2ex;
     padding-left: 2px;
     padding-right: 2px;
 }

Also applies to: 42-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/image-gallery.css` around lines 23 - 29, The selector
".image-gallery gallery-object a figcaption" is invalid because figcaption is a
sibling of <a>, so its rules never apply; move the typography declarations
(font-size, overflow, padding, text-align, text-wrap) into the existing
"gallery-object figcaption" rule (and also update the duplicate block around
lines 42-47) so the figcaption styling is applied to the correct element; update
only the selector-targeted rule names ("gallery-object figcaption") and remove
the unreachable ".image-gallery gallery-object a figcaption" block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/image-gallery.css`:
- Around line 62-68: The .gallery.page_menu block is still visually off-center
because it has a fixed width: 80% without centering the block itself; update the
.gallery.page_menu rule to center the element by adding margin-inline: auto (or
remove the fixed width and use width: 100% with a max-width if preferred) so the
flex children remain centered within a centered container.

---

Duplicate comments:
In `@website/assets/css/image-gallery.css`:
- Around line 23-29: The selector ".image-gallery gallery-object a figcaption"
is invalid because figcaption is a sibling of <a>, so its rules never apply;
move the typography declarations (font-size, overflow, padding, text-align,
text-wrap) into the existing "gallery-object figcaption" rule (and also update
the duplicate block around lines 42-47) so the figcaption styling is applied to
the correct element; update only the selector-targeted rule names
("gallery-object figcaption") and remove the unreachable ".image-gallery
gallery-object a figcaption" block.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 15da4fd5-bc73-48ad-8787-18582b961012

📥 Commits

Reviewing files that changed from the base of the PR and between 87e26c8 and 6d79177.

📒 Files selected for processing (1)
  • website/assets/css/image-gallery.css

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
website/assets/css/image-gallery.css (1)

62-68: 🧹 Nitpick | 🔵 Trivial

.gallery.page_menu block may appear visually off-center (duplicate).

This issue was previously flagged: with width: 80% but no margin-inline: auto, the block can sit left-aligned while only its children are centered.

♻️ Suggested fix
 .gallery.page_menu {
     display: flex;
     flex-direction: row;
     justify-content: center;
     align-items: center;
     width: 80%;
+    margin-inline: auto;
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/image-gallery.css` around lines 62 - 68, The
.gallery.page_menu rule centers its children but the container itself is set to
width: 80% without centering, causing the block to appear visually off-center;
update the .gallery.page_menu style to center the container itself (e.g., add
margin-inline: auto or equivalent centering such as margin: 0 auto) or adjust
the width to auto so the container is naturally centered while keeping the
existing flex centering of its children.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/assets/css/image-gallery.css`:
- Line 68: Remove the inconsistent leading space before the closing brace
currently written as ' }' in the CSS diff: replace the ' }' with '}' so the
closing brace matches the rest of the file (no leading whitespace), ensuring all
CSS rule closures use '}' consistently.

---

Duplicate comments:
In `@website/assets/css/image-gallery.css`:
- Around line 62-68: The .gallery.page_menu rule centers its children but the
container itself is set to width: 80% without centering, causing the block to
appear visually off-center; update the .gallery.page_menu style to center the
container itself (e.g., add margin-inline: auto or equivalent centering such as
margin: 0 auto) or adjust the width to auto so the container is naturally
centered while keeping the existing flex centering of its children.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8fa71ae8-d9ce-4ce5-a383-877d2f0a32a8

📥 Commits

Reviewing files that changed from the base of the PR and between 87e26c8 and 6d79177.

📒 Files selected for processing (1)
  • website/assets/css/image-gallery.css

justify-content: center;
align-items: center;
width: 80%;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Inconsistent spacing before closing brace.

Line 68 has a leading space ( }), while all other closing braces in the file have no whitespace. Consider removing the space for consistency.

♻️ Proposed fix
     width: 80%;
-    margin-inline: auto;
- }
+    margin-inline: auto;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}
.gallery.page_menu {
display: flex;
flex-direction: row;
justify-content: center;
align-items: center;
width: 80%;
margin-inline: auto;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/assets/css/image-gallery.css` at line 68, Remove the inconsistent
leading space before the closing brace currently written as ' }' in the CSS
diff: replace the ' }' with '}' so the closing brace matches the rest of the
file (no leading whitespace), ensuring all CSS rule closures use '}'
consistently.

@dstopp dstopp requested a review from rfrenchseti April 7, 2026 19:31
@dstopp dstopp merged commit e3255d9 into main Apr 7, 2026
1 check passed
@dstopp dstopp deleted the 210_update_galleries branch April 7, 2026 19:39
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