feat(theme): add local custom CSS and logos #4021
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughConfiguration endpoint shifted from external production URL to local static path. Theme-specific CSS files added for multiple views (Aoste, Fictive, Global, Highlands) with consistent styling patterns. ARIA labels added to templates for accessibility. Test updated to validate local endpoint. Minor code style formatting applied to scripts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@rero_ils/config.py`:
- Around line 278-280: The comment above the
RERO_ILS_THEME_ORGANISATION_CSS_ENDPOINT config is outdated; update or remove
the line "For production: change 'test' with 'prod' on url" to reflect that the
endpoint is now a local static path (RERO_ILS_THEME_ORGANISATION_CSS_ENDPOINT =
"/static/themes/css/") or replace it with a brief note stating that this is
served from local static files in production.
In `@rero_ils/theme/static/themes/css/aoste.css`:
- Around line 15-17: The CSS rule currently targets the wrong view selector
("#view-global .rero-slogan") so the slogan styles don't apply to the Aoste
organisation view; update the selector to target the Aoste view by replacing the
selector "#view-global .rero-slogan" with "#view-aoste .rero-slogan" in the
aoste.css file so the margin-bottom: 4em rule applies to the correct element.
- Around line 29-35: The responsive rules are scoped to the wrong container:
replace selectors using "#view-global .rero-ils-frontpage" and "#view-global
.rero-slogan" with the "#view-aoste" scope so the styles apply to the Aoste view
(update occurrences of "#view-global .rero-ils-frontpage" and "#view-global
.rero-slogan" to "#view-aoste .rero-ils-frontpage" and "#view-aoste
.rero-slogan" respectively).
In `@rero_ils/theme/static/themes/css/fictive.css`:
- Around line 15-17: The CSS rule in fictive.css is targeting the wrong view id:
replace the selector target "#view-global .rero-slogan" with the fictive view id
"#view-fictive .rero-slogan" so the margin-bottom: 4em applies to the fictive
organisation view; update the selector in the rule that contains ".rero-slogan"
to use "#view-fictive" instead of "#view-global".
- Around line 29-35: The responsive rules currently target `#view-global` but
should apply to the fictive view — update the selectors that reference
`#view-global` to `#view-fictive` so the styles for .rero-ils-frontpage and
.rero-slogan apply on the fictive view (i.e., change occurrences of
"#view-global .rero-ils-frontpage" and "#view-global .rero-slogan" to
"#view-fictive .rero-ils-frontpage" and "#view-fictive .rero-slogan").
In `@rero_ils/theme/static/themes/css/highlands.css`:
- Around line 15-17: The CSS rule currently uses the wrong selector: replace the
selector "#view-global .rero-slogan" with "#view-highlands .rero-slogan" so the
slogan styles apply to the Highlands organisation view; update the rule in
highlands.css where the selector for .rero-slogan is defined (keep the same
margin-bottom: 4em declaration).
- Around line 29-35: The CSS selectors currently target `#view-global` (e.g.,
selectors "#view-global .rero-ils-frontpage" and "#view-global .rero-slogan")
but should target the highlands view; update those selectors to use
"#view-highlands" instead of "#view-global" so the responsive rules apply to the
correct view (replace any occurrence of "#view-global" in these rules with
"#view-highlands" and keep the existing declarations intact).
rerowep
force-pushed
the
wep-custom-css
branch
2 times, most recently
from
702640f to
20bbcf6
Compare
PascalRepond
approved these changes
Feb 2, 2026
rerowep
force-pushed
the
wep-custom-css
branch
from
20bbcf6 to
c19b4a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@rero_ils/theme/static/themes/css/fictive.css`:
- Around line 10-13: The rule using content on div#fictive-logo has no effect;
change to either use a pseudo-element or a background image: replace the current
declaration by adding a div#fictive-logo::before rule that sets content: '' and
uses background-image: url('/static/themes/images/logo-fictive.svg') with
background-size: contain/cover, background-repeat: no-repeat and appropriate
display/width/height (or alternatively set the image as background-image on
div#fictive-logo itself and ensure display and size), so the SVG actually
renders; adjust max-height into the element/pseudo-element sizing as needed.
In `@tests/ui/test_views.py`:
- Around line 83-97: Update the test_external_endpoint_on_institution_homepage
test to first assert result.status_code == 200 to avoid false positives from
redirects/errors, and use the decoded response text by checking endpoint in
result.text instead of in str(result.data); reference the test function name
test_external_endpoint_on_institution_homepage and the variables result and
endpoint when making these changes.
c19b4a7 to
aa707d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@rero_ils/theme/static/themes/css/aoste.css`:
- Around line 10-13: The CSS uses content: url() on a regular element
(div#aoste-logo) which is ignored; change the rule to target the pseudo-element
(div#aoste-logo::before), move the url into that pseudo-element, and add
display: block (and any needed width/height or max-height) to ensure the SVG
renders and sizes correctly; update any existing selectors that rely on the old
rule to use the pseudo-element instead.
In `@rero_ils/theme/static/themes/css/highlands.css`:
- Around line 10-13: The CSS uses content on a normal element
(div#highlands-logo) which is unreliable; change to a generated pseudo-element
instead: remove the content property from div#highlands-logo and add a rule for
div#highlands-logo::before that sets content:
url('/static/themes/images/logo-highlands.svg'), appropriate display (e.g.,
inline-block or block), and the max-height: 44px (or apply max-height to the
pseudo-element) so the SVG renders consistently; apply the same pattern for the
other theme logo selectors (fictive, aoste, global) to replace
content-on-element with ::before generated-content.
5c69d80 to
fed8b4f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@rero_ils/theme/static/themes/css/aoste.css`:
- Around line 21-28: The responsive heading rules are currently global because
they use .h1, h1 and .h2, h2 selectors; restrict them to the aoste view by
prefixing the selectors with the aoste view root (e.g. change selectors to
target .aoste .h1, .aoste h1 and .aoste .h2, .aoste h2 or the actual view root
class used in the template), so the media query only affects headings inside the
aoste view; update both occurrences in the media query block that contains .h1,
h1 and .h2, h2.
In `@rero_ils/theme/static/themes/css/global.css`:
- Around line 22-29: The responsive media query currently targets global
selectors `.h1, h1` and `.h2, h2`, affecting all pages; scope these rules to
only apply within the view-global container by qualifying selectors (e.g.,
`view-global .h1`, `view-global h1`, `view-global .h2`, `view-global h2`) so
that the font-size changes inside the `@media (max-width: 576px)` block only
affect the intended view-global context.
🧹 Nitpick comments (3)
rero_ils/theme/static/themes/css/global.css (2)
10-16: Size constraints may not work as expected oncontent: url()images.When using
content: url()to insert an image, the pseudo-element becomes a replaced element. Properties likemin-height,max-height, andmin-widthmay not constrain the actual image size as expected—the image renders at its intrinsic dimensions. Consider usingwidth/heightwithobject-fit, or control the SVG'sviewBoxdimensions directly.Alternative approach using background-image
div#global-logo::before { - content: url('/static/themes/images/logo-global.svg'); - min-height: 44px; - max-height: 44px; - min-width: 58px; + content: ''; + background-image: url('/static/themes/images/logo-global.svg'); + background-size: contain; + background-repeat: no-repeat; + width: 58px; + height: 44px; display: block; }
1-39: Inconsistent indentation throughout the file.The file mixes 2-space indentation (lines 2-16) with 4-space indentation (lines 19, 23-37). Consider normalizing to a consistent style for maintainability.
rero_ils/theme/static/themes/css/aoste.css (1)
10-14: Logo sizing is inconsistent with global.css pattern.The
global.cssdefinesmin-height,max-height, andmin-widthfor its logo, while this file only hasmax-height. Consider aligning the sizing approach across theme files for consistency, or verify that the aoste SVG has appropriate intrinsic dimensions.Align with global.css pattern
div#aoste-logo::before { content: url('/static/themes/images/logo-aoste.svg'); + min-height: 44px; max-height: 44px; + min-width: 58px; display: block; }
rerowep
force-pushed
the
wep-custom-css
branch
from
fed8b4f to
d8db911
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@rero_ils/theme/static/themes/css/fictive.css`:
- Around line 6-14: Hiding body#view-fictive nav.rero-ils-header
img.rero-ils-logo and replacing it with div#fictive-logo::before removes the
image's accessible name and leaves the parent <a id="homepage-logo"> unlabeled;
fix by adding an accessible name to the link (e.g., add
aria-label="{{_(config.THEME_SITENAME)}} logo" on the <a id="homepage-logo"> in
header.html) or by restoring the DOM-based label (move the SVG into a visually
hidden <span> or an <img> inside the <a> and remove the display:none on
img.rero-ils-logo); apply the same change across all theme CSS/headers (fictive,
highlands, aoste, global).
d8db911 to
4e3b1fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/load_testing.py`:
- Around line 110-119: The success and error prints in the processing loop use
inconsistent indexing: the success message uses index+1 but the exception
message uses index; update the exception print to use the same 1-based index by
changing the reference from index to index+1 in the except block (look for the
try/except that prints "Updated {url} in execution ..." and the corresponding
except that prints "Error during processing of {url} in execution {index} ..." —
modify that except print to use index+1 and keep self.thread_number unchanged).
🧹 Nitpick comments (1)
rero_ils/theme/static/themes/css/highlands.css (1)
22-28: Responsive heading styles lack view-specific scoping.The
.h1, h1and.h2, h2rules are not scoped to#view-highlandsand will affect all views when this CSS is loaded. Scope them to avoid unintended side effects on other pages.♻️ Proposed fix to scope selectors
`@media` (max-width: 576px) { - .h1, h1 { + `#view-highlands` .h1, `#view-highlands` h1 { font-size: 1.5rem; } - .h2, h2 { + `#view-highlands` .h2, `#view-highlands` h2 { font-size: 1.2rem; }
rerowep
force-pushed
the
wep-custom-css
branch
2 times, most recently
from
2bb0cf5 to
960e3f4
Compare
rerowep
force-pushed
the
wep-custom-css
branch
2 times, most recently
from
dfe8412 to
62be6d7
Compare
Add organisation-specific theming support with local static assets instead of remote CDN. * Add custom CSS files for aoste, fictive, global, and highlands organisations * Add corresponding logo SVG files for each organisation * Configure RERO_ILS_THEME_ORGANISATION_CSS_ENDPOINT to use local /static/themes/css/ endpoint Each organisation CSS customizes header background colors, logo display, and responsive layouts for mobile devices. Co-Authored-by: Peter Weber <[email protected]>
rerowep
force-pushed
the
wep-custom-css
branch
from
62be6d7 to
b05c078
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add organisation-specific theming support with local static assets instead of remote CDN.
Each organisation CSS customizes header background colors, logo display, and responsive layouts for mobile devices.