Update websit to include nav bar, readme site and load content from readme.md - #63
Conversation
…ustments to frontend aspects, add readme.html to host readme.md, basic javascript for loading markdown language content.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a top navigation and Read Me page, SPA-like client routing with history and fade transitions, client-side Markdown rendering, a new README describing a filter pipeline, and extensive CSS/markup updates for layout and styling. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant AppJS as "app.js (router)"
participant Server
participant Marked as "marked.parse (client)"
Browser->>AppJS: click link to /readme.html
AppJS->>Browser: trigger fade-out, disable anchors
AppJS->>Server: GET /readme.html
Server-->>AppJS: respond with readme.html
AppJS->>Browser: replace <main>, pushState, trigger fade-in, enable anchors
AppJS->>Server: GET /README.md
Server-->>AppJS: return README.md
AppJS->>Marked: marked.parse(README.md)
Marked-->>AppJS: rendered HTML
AppJS->>Browser: inject into `#readme_content`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 9
🤖 Fix all issues with AI agents
In `@src/main/resources/static/css/styles.css`:
- Around line 222-228: The selector "& ul > li" contains two conflicting margin
declarations—"margin: 0.25rem 1rem" and later "margin: 0"—so remove the dead
declaration (delete the earlier or later margin depending on intended spacing)
in the rules for "& ul > li" to keep only the correct margin; update the
selector block (the "& ul > li" rule) so it has a single, intended margin value
and ensure no other properties rely on the removed value.
- Around line 177-196: Merge the two &::after blocks into a single rule so the
positioning, border-radius, position-anchor, transition and the visual
properties (background-color, z-index) are applied together; locate the
duplicate &::after selector and consolidate its declarations into one block,
remove the redundant second block, and while merging fix the transition
declaration syntax (add the missing semicolon and ensure a valid timing-function
or use a standard easing function) so the rule for &::after (including
position-anchor: --hovered-feature) is valid and not silently overridden.
- Around line 291-304: The footer rule uses rgba(var(--base-color-white), 0.9)
which is invalid because --base-color-white is a hex string; update the footer
selector to either use the variable directly (color: var(--base-color-white)) or
introduce a separate RGB variable (e.g., --base-color-white-rgb) and reference
that with rgba(var(--base-color-white-rgb), 0.9); change the declaration in your
root variables and adjust the footer rule accordingly.
In `@src/main/resources/static/index.html`:
- Around line 28-29: Move the <h2>Features</h2> element so it is nested inside
the <section class="features"> element (i.e., place the heading as the first
child of the section) to restore proper document semantics and outline for
assistive technologies; update the markup around the <section class="features">
block (look for the existing <h2>Features</h2> and <section class="features">)
to ensure the heading is contained within that section tag.
In `@src/main/resources/static/js/markdown.js`:
- Around line 1-6: Check response.ok after the fetch("/readme.md") call and only
call response.text() and marked.parse(markdown) when response.ok is true; if not
ok, avoid injecting the error HTML into the DOM (the sequence involving
response, .text(), marked.parse, and container.innerHTML in the current fetch
chain) and instead handle the error path by logging the response status and
showing a safe fallback message in the element obtained via
document.getElementById("readme_content").
In `@src/main/resources/static/readme.html`:
- Line 18: Fix the malformed numeric character references by adding the missing
semicolons to the HTML entities 🔍 and 🔎 in the readme header;
locate the string "<h1>🔍 ReadMe 🔎</h1>" and change the entities to
"🔍" and "🔎" so they become valid HTML numeric character
references.
- Around line 28-30: The README content fails to load on case-sensitive
filesystems because markdown.js fetches "/readme.md" while the actual static
file is named "README.md"; update the fetch URL in markdown.js to "/README.md"
(or alternatively rename the static file to "readme.md") so the path matches
exactly, and verify the fetch call in markdown.js (the fetch(...) invocation)
now succeeds without 404s.
- Around line 38-39: The page is broken on case-sensitive filesystems because
markdown.js fetches "/readme.md" but the actual file is "README.md" and the
script tags include marked.min.js and markdown.js; update the fetch path in
markdown.js to "/README.md" (or rename the file to readme.md) so the resource
loads on Linux, and also fix the two HTML entities in readme.html (replace
🔍 and 🔎 with 🔍 and 🔎) so they render correctly.
In `@src/main/resources/static/README.md`:
- Line 1: Replace the placeholder top-level heading "# PLACEHOLDER" in README.md
with a meaningful document title such as "# Filter System Documentation" (or
another concise, descriptive title relevant to the project) so the file header
accurately reflects its contents and purpose.
🧹 Nitpick comments (5)
src/main/resources/static/js/markdown.js (1)
3-5: Consider sanitizing the parsed Markdown before injecting viainnerHTML.
marked.parsedoes not sanitize output by default. While the content is a same-origin static file today, injecting unsanitized HTML viainnerHTMLis an XSS vector if the source ever changes. Consider using a sanitizer like DOMPurify, or enablingmarked's sanitization options.src/main/resources/static/README.md (1)
37-40: Add a language identifier to the fenced code block.The code block starting at line 37 is missing a language specifier. Since this is Java code, annotate it as such for proper syntax highlighting in the rendered output.
Proposed fix
-``` +```java Pipeline pipeline = new Pipeline(); pipeline.addFilter(new LoggingFilter());</details> </blockquote></details> <details> <summary>src/main/resources/static/css/styles.css (3)</summary><blockquote> `30-48`: **Duplicate `background` and `border-radius` declarations in `.container`.** Line 34 sets `background: transparent` and line 37 overrides it with a gradient. Line 35 sets `border-radius: 10px` and line 40 re-declares the same value. The first declarations are dead code. <details> <summary>Proposed cleanup</summary> ```diff .container { max-width: 900px; margin: 0 auto; - background: transparent; - border-radius: 10px; - background: linear-gradient(135deg, rgba(23, 23, 46, 0.6), rgba(23, 23, 46, 0.9), rgba(23, 23, 46, 0.6)); color: white; text-align: center; border-radius: 10px;
282-288: Remove commented-out code.The old
.features liblock is commented out. If it's no longer needed, remove it to keep the stylesheet clean. Version control preserves the history if it's ever needed again.
2-4: Custom properties on*selector — prefer:root.Defining CSS custom properties on the universal selector
*causes them to be set on every element in the DOM. The conventional and more efficient approach is to define them on:root.Proposed fix
-* { - --base-color: 23, 23, 46; - --base-color-white: `#f7f7f7fc`; +:root { + --base-color: 23, 23, 46; + --base-color-white: `#f7f7f7fc`; +} + +* { margin: 0; padding: 0; box-sizing: border-box; }
…oothed out transitions between sites from navigation.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/main/resources/static/css/styles.css`:
- Around line 34-41: Remove the dead duplicate CSS declarations: in the rule
block that currently contains both "background: transparent;" and
"border-radius: 10px" followed later by "background: linear-gradient(...)" and
another "border-radius: 10px", delete the earlier "background: transparent;" and
the first "border-radius: 10px" so only the intended gradient background
(background: linear-gradient(...)) and a single border-radius remain; update the
same rule where backdrop-filter and color are defined (the block containing
backdrop-filter: blur(4px), color: white, text-align: center) to ensure no
duplicate property lines remain.
- Around line 58-170: The nav hover decorations rely solely on Anchor
Positioning (anchor-name, position-anchor, anchor()) which breaks on older
browsers; add non-anchor fallbacks by defining simple hover styling outside the
anchor-dependent rules (e.g., a background-color or box-shadow on .nav-menu ul >
li a:hover) and use the existing .disable-anchors class as a toggle to apply
those fallbacks when anchors aren't supported; update the
&:has(a:hover)::before/::after anchor rules to be wrapped by an
`@supports`(anchor-name: --x) feature query (or inverse `@supports` not(...) to
apply fallbacks) so .nav-menu, .nav-menu ul > li and .nav-menu ul > li a have a
clear, anchor-free hover style for older browsers.
In `@src/main/resources/static/js/app.js`:
- Around line 69-79: The error message in initReadme() is currently Swedish;
update the fetch error handling to use an English message. Locate the initReadme
function and replace the console.error call in the .catch(...) that currently
logs "Kunde inte ladda README.md" so it logs an English message like "Failed to
load README.md" (including the err object) to match the rest of the UI and
codebase.
- Around line 58-66: The current click binding uses
document.querySelectorAll("a") at module load so links injected later (after
navigate() updates main.innerHTML) lack handlers; change to event delegation by
adding a single click listener on the document or the container element that
holds the page content (e.g., the same element updated by main.innerHTML), and
inside the handler use event.target.closest('a') to find the clicked link, check
its href (endsWith(".html")), call event.preventDefault() and invoke
navigate(href) for those links so dynamically inserted anchors are handled.
- Around line 7-14: Add a popstate listener after DOMContentLoaded that calls
your existing route(window.location.pathname) handler so back/forward navigation
updates content; in that listener do NOT call navigate() (to avoid duplicate
history.pushState) — instead have navigate(path, {replaceState:false}) or accept
a flag like navigate(path, {fromPopstate:true}) and check it before calling
history.pushState, or detect history.state to skip pushState; reference the
route and navigate functions and the popstate event to locate where to add the
listener and the guard.
- Around line 37-56: The navigate() implementation lacks error handling for the
fetch(href) call and removes the nav "disable-anchors" class on a fixed timeout,
causing permanent blank pages on network errors; update the fetch chain used in
navigate() to add a .catch() (or try/catch around await) that restores the
original main content and removes the "fade-out" class on error, logs the error,
and ensures history.pushState only runs on success, and move the
nav.classList.remove("disable-anchors") call into the fetch completion path
(then/finally) so it is removed after the navigation completes or fails rather
than after a fixed 300ms; reference functions/identifiers: navigate(),
fetch(href), main, main.classList.remove("fade-out"),
nav.classList.remove("disable-anchors"), route(href), history.pushState.
🧹 Nitpick comments (3)
src/main/resources/static/css/styles.css (2)
3-4: Custom properties on*— prefer:root.Defining custom properties on the universal selector means they're set on every element. Convention is to use
:rootfor custom properties so they're defined once and inherited everywhere, which is also slightly more performant.
129-151: Large block of commented-out CSS.~23 lines of commented-out
:activestyles and the.features liblock (lines 318–324) add noise. Remove dead code and rely on version control to recover it if needed.src/main/resources/static/index.html (1)
46-48: Remove commented-out script tag.Line 47 is a commented-out
<script>include formarkdown.js. If it's not needed, remove it rather than leaving dead code in the HTML.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/resources/static/css/styles.css`:
- Around line 25-32: The global rule "a { display: block; ... }" should not
apply to all anchors because it breaks inline README links; change that selector
to target only navigation anchors (e.g., use ".nav a" or a dedicated class
selector like ".nav-link") and leave the rest of the declarations (padding,
color, font-weight, font-family) intact; update the relevant navigation markup
to add the chosen class (if you choose .nav-link) so only nav links become
block-level while regular inline links remain unaffected.
In `@src/main/resources/static/js/app.js`:
- Around line 42-44: The fetch flows in navigate() and initReadme() call
res.text() unconditionally and may inject HTML from error pages; update both
functions to check the Response.ok (or response.status) before calling
res.text(): if (!res.ok) throw a descriptive Error including
res.status/res.statusText (or read res.text() for richer message) so the promise
rejects and is handled, and ensure the surrounding .catch or error branch
displays a safe error message instead of injecting raw error-page HTML.
🧹 Nitpick comments (1)
src/main/resources/static/css/styles.css (1)
134-156: Remove commented-out code block before merging.This large block of commented-out CSS (
:activestyles) adds noise. If it's experimental/WIP, track it in an issue instead of leaving it in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main/resources/static/css/styles.css`:
- Around line 183-184: In the CSS rule that currently has "border-top: solid
`#f7f7f717`" followed by "border-width: 1px", remove the conflicting shorthand and
replace them with a single explicit declaration such as "border-top: 1px solid
`#f7f7f717`" (or alternatively change "border-width: 1px" to "border-top-width:
1px") so only the top border is affected; apply the same fix to the footer rule
that uses the same pattern.
In `@src/main/resources/static/js/app.js`:
- Around line 7-14: The code queries const nav =
document.querySelector(".nav-menu") at module load which can be null on pages
without that element; update navigate() (or move the query inside it) to
re-query nav and guard usage by checking nav !== null before calling
nav.classList.add/remove (or early-return if missing) so navigate() never
dereferences a null nav; reference the document.querySelector(".nav-menu") call
and the navigate() function when making this change.
- Around line 85-98: The initReadme function currently sets container.innerHTML
= marked.parse(md) which injects unsanitized HTML and permits XSS; update
initReadme to sanitize the parsed HTML before assigning it (e.g., use
DOMPurify.sanitize(marked.parse(md)) or enable marked's sanitizer option when
configuring marked), ensuring you reference the container element and
marked.parse(md) call; also remove or consolidate the redundant markdown.js dead
file (or uncomment and integrate it consistently) so there aren’t duplicate
unsafe implementations.
🧹 Nitpick comments (1)
src/main/resources/static/css/styles.css (1)
134-156: Remove commented-out code blocks.Lines 134–156 contain a large commented-out CSS block for
:activestates, and lines 322–328 comment out the old.features lirule. These add noise and should be removed. If needed later, they can be recovered from version control.Also applies to: 322-328
…ncy min.js and its callback in index.html and readme.html. Removed superfluous markdown.js.
simonforsberg
left a comment
There was a problem hiding this comment.
Nicely done!
Question, will this README.md replace the one from PR #62?
Do you mean when we merge? I do not believe they are located in the same folder, so it should be fine for the merge, and we will simply migrate the content of the updated readme to this file for pathing. |
Closes #60
Update index.html to include a nav bar at the top, styles.css for adjustments to frontend aspects, add readme.html to host readme.md, basic javascript for loading markdown language content.
Summary by CodeRabbit
New Features
Documentation
Style