Conversation
…pdate_galleries
…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.
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds 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 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ 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. Comment |
There was a problem hiding this comment.
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).
… pages. Includes css changes" This reverts commit 0925e3b.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
rfrenchseti
left a comment
There was a problem hiding this comment.
I'm not qualified to comment on this PR, except to note that it's not clear if the CodeRabbit issues have been addressed.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
website/assets/css/style.css (2)
791-793:⚠️ Potential issue | 🟠 MajorSubnav keyboard accessibility still unaddressed —
:focus-withinexclusion missing.This was raised in a previous review and remains unresolved. Two problems persist:
- 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.- When a keyboard user tabs into a subnav descendant,
:hoveris 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: autostill won't centera.press_release img—display: blockis missing.This was raised in a previous review and remains unaddressed. Without
display: block,imgstays inline andmargin: autohas 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.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
website/_includes/image-gallery.html (1)
1-17: 🧹 Nitpick | 🔵 TrivialMove 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.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
website/_plugins/div_tag.rb (1)
9-13:⚠️ Potential issue | 🔴 Critical
rendercurrently has a runtime break and unsafe interpolation.At Line 12,
converteris 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 | 🟠 MajorRemove duplicate inline styles now that
image-gallery.cssexists.These inline styles are now duplicated in
website/assets/css/image-gallery.css, which is loaded viahead.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
📒 Files selected for processing (5)
website/_includes/head.htmlwebsite/_includes/image-gallery.htmlwebsite/_plugins/div_tag.rbwebsite/assets/css/image-gallery.csswebsite/assets/css/style.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
website/_includes/image-gallery.html (1)
10-13:⚠️ Potential issue | 🟡 MinorUse truthy checks for
image.hrefandimage.src(not!= nil).Line 10 and Line 12 still use
!= nil; in Liquid this can treat explicitfalseas 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
galleryselector 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
📒 Files selected for processing (5)
website/_includes/head.htmlwebsite/_includes/image-gallery.htmlwebsite/_plugins/div_tag.rbwebsite/assets/css/image-gallery.csswebsite/assets/css/style.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
website/_plugins/div_tag.rb (1)
8-17:⚠️ Potential issue | 🔴 CriticalCritical: Fatal syntax errors prevent this plugin from loading.
This file contains multiple syntax errors that will cause Ruby to fail at parse time:
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.Shell command execution via backticks (line 13):
`@class`executes@classas a shell command. Use#{@Class}for string interpolation.Missing module
end(line 15): Theendcloses theDivTagclass, but theJekyllmodule opened on line 1 is never closed.RuboCop confirms:
unexpected token kENDat line 14 andunexpected token $endat 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
📒 Files selected for processing (5)
website/_includes/head.htmlwebsite/_includes/image-gallery.htmlwebsite/_plugins/div_tag.rbwebsite/assets/css/image-gallery.csswebsite/assets/css/style.css
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
website/_plugins/div_tag.rb (1)
8-13:⚠️ Potential issue | 🔴 CriticalFix the malformed
rendermethod 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
📒 Files selected for processing (6)
website/_includes/gallery.htmlwebsite/_includes/head.htmlwebsite/_includes/image-gallery.htmlwebsite/_plugins/div_tag.rbwebsite/assets/css/image-gallery.csswebsite/assets/css/style.css
matthewtiscareno
left a comment
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
website/_includes/image-gallery.htmlwebsite/assets/css/image-gallery.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
website/_includes/image-gallery.html (1)
10-17:⚠️ Potential issue | 🟠 Major
image.hrefbranch still uses a different thumbnail path and can regress previews.
Line 12builds{{ site.assets_url }}{{ image.src }}, whileLine 16uses the preview path prefix. Ifimage.srcis 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 figcaptionselector does not match current markup.In
website/_includes/image-gallery.html,<figcaption>is outside<a>, soLines 23-29never 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
📒 Files selected for processing (2)
website/_includes/image-gallery.htmlwebsite/assets/css/image-gallery.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
website/assets/css/image-gallery.css (1)
23-29:⚠️ Potential issue | 🟠 Major
figcaptionselector does not match the current gallery markup.
website/_includes/image-gallery.htmlplaces<figcaption>outside<a>, so.image-gallery gallery-object a figcaptionnever 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
📒 Files selected for processing (1)
website/assets/css/image-gallery.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
website/assets/css/image-gallery.css (1)
23-29:⚠️ Potential issue | 🟠 MajorFigcaption typography rule is unreachable with current DOM.
figcaptionis a sibling of<a>, not a descendant, so the selector on Line 23 never matches. Move those declarations to the existinggallery-object figcaptionrule 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
📒 Files selected for processing (1)
website/assets/css/image-gallery.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
website/assets/css/image-gallery.css (1)
62-68: 🧹 Nitpick | 🔵 Trivial
.gallery.page_menublock may appear visually off-center (duplicate).This issue was previously flagged: with
width: 80%but nomargin-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
📒 Files selected for processing (1)
website/assets/css/image-gallery.css
| justify-content: center; | ||
| align-items: center; | ||
| width: 80%; | ||
| } |
There was a problem hiding this comment.
🧹 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.
| } | |
| .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.
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