Feature/landing page polish#110
Merged
Merged
Conversation
📝 WalkthroughWalkthroughA new Changes
Sequence DiagramsequenceDiagram
actor User
participant UiController
participant PublicStatsService
participant Repositories as CaseRepository<br/>PatientRepository<br/>EmployeeRepository
participant View as landing.html
User->>UiController: GET /
UiController->>PublicStatsService: getPublicStats()
PublicStatsService->>Repositories: count() × 3
Repositories-->>PublicStatsService: totalCases, totalPatients,<br/>totalEmployees
PublicStatsService-->>UiController: Map with stats
UiController->>UiController: model.addAttribute("stats", ...)
UiController->>View: render landing view
View-->>User: landing page with metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/org/example/projektarendehantering/application/service/PublicStatsService.java (1)
21-28: Cache the public stats; consider returning an immutable map.
getPublicStats()is called on every hit to the publicGET /landing page (seeUiController.landing) and issues three full table COUNT queries each time. For an unauthenticated, presumably high-traffic page this is unnecessary load on the DB and slows TTFB. A short TTL cache (Spring@Cacheablewith Caffeine, or a manualAtomicReference+ scheduled refresh) is a much better fit since the numbers don't need to be real-time.Also, since callers (Thymeleaf) only read the result, returning an immutable
Map.of(...)makes the intent clearer and avoids accidental mutation.♻️ Suggested refactor
- `@Transactional`(readOnly = true) - public Map<String, Long> getPublicStats() { - Map<String, Long> stats = new HashMap<>(); - stats.put("totalCases", caseRepository.count()); - stats.put("totalPatients", patientRepository.count()); - stats.put("totalEmployees", employeeRepository.count()); - return stats; - } + `@Transactional`(readOnly = true) + `@Cacheable`("publicStats") + public Map<String, Long> getPublicStats() { + return Map.of( + "totalCases", caseRepository.count(), + "totalPatients", patientRepository.count(), + "totalEmployees", employeeRepository.count() + ); + }(Requires
@EnableCachingon a config class and a cache provider; also schedule eviction, e.g., every 1–5 minutes.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/application/service/PublicStatsService.java` around lines 21 - 28, getPublicStats currently issues three COUNT queries on every public hit (used by UiController.landing); wrap it with a short TTL cache (e.g., annotate PublicStatsService.getPublicStats with Spring's `@Cacheable` backed by Caffeine and ensure `@EnableCaching` is present) so the DB counts are served from cache for 1–5 minutes, or alternatively use a cached AtomicReference with a scheduled refresh; also return an immutable map (e.g., Map.of(...)) instead of a mutable HashMap to prevent accidental mutation by callers.src/main/resources/static/app.js (1)
224-235: Reflow trick currently has no visible effect.The remove-class → force-reflow → re-add-class pattern is meant to restart a CSS transition/animation tied to
.is-active. Looking at.flow-detail(and analogously.role-panelat lines 271-282) inapp.css, neither selector defines a transition or animation that keys offis-active, so this code path adds complexity without visible payoff. Either drop the toggle or add anis-activetransition (e.g., fade/translate) to actually use it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/static/app.js` around lines 224 - 235, The remove-class → void flowDetail.offsetWidth → add-class pattern around flowDetail (and the analogous rolePanel code) currently has no effect because .flow-detail and .role-panel lack any CSS transition/animation keyed to the .is-active class; either remove the DOM class-toggle + forced reflow (delete the flowDetail.classList.remove / void flowDetail.offsetWidth / flowDetail.classList.add lines) to simplify the code, or add CSS animations for .flow-detail.is-active and .role-panel.is-active (e.g., add transition + transform/opacity rules for .flow-detail and .role-panel, and a different state for .is-active) so the existing flowDetail and rolePanel class toggles actually trigger a visible fade/translate animation. Ensure you update both the JS references (flowDetail, rolePanel) and the CSS selectors (.flow-detail.is-active, .role-panel.is-active) consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/static/app.css`:
- Around line 1175-1178: There are two conflicting `@keyframes` declarations named
`panel-glow`; remove the duplicate by either deleting the earlier definition or
renaming this second one to a distinct name (e.g., `panel-glow-variant`) and
update any CSS rules that reference `animation-name`/`animation` using
`panel-glow` to the new name so the intended animation is applied; make the
change in the `@keyframes panel-glow` block and any selectors that consume it
(search for `panel-glow` usages) to keep behavior consistent.
- Around line 1088-1115: The new large-card styles unintentionally override the
base .metric-chip used across the site; scope the hero-only styling instead of
replacing .metric-chip by either (A) prefixing the rules with .metrics-row--hero
.metric-chip or (B) creating a modifier like .metric-chip--hero and moving the
large-card rules into that selector (and updating .metric-icon and
.metric-content selectors to .metric-chip--hero .metric-icon /
.metric-chip--hero .metric-content). Restore the original small-pill properties
on .metric-chip (min-height: 28px; border-radius: 999px; padding: 0 10px;
font-size: 12px) so non-hero chips keep their intended appearance, and update
any hero markup to use the new modifier if you choose option B.
In `@src/main/resources/templates/landing.html`:
- Around line 19-41: The template currently uses literal "0" inside the <strong>
tags as a static preview, but that does not serve as a runtime fallback; update
the three <strong> elements that use th:text (the ones bound to
stats.totalCases, stats.totalPatients, and stats.totalEmployees) to use the
Thymeleaf/OGNL Elvis operator in their th:text expressions so that they render 0
when the stats value is null or missing (replace the current th:text expressions
on those <strong> elements with an Elvis-style fallback producing 0).
---
Nitpick comments:
In
`@src/main/java/org/example/projektarendehantering/application/service/PublicStatsService.java`:
- Around line 21-28: getPublicStats currently issues three COUNT queries on
every public hit (used by UiController.landing); wrap it with a short TTL cache
(e.g., annotate PublicStatsService.getPublicStats with Spring's `@Cacheable`
backed by Caffeine and ensure `@EnableCaching` is present) so the DB counts are
served from cache for 1–5 minutes, or alternatively use a cached AtomicReference
with a scheduled refresh; also return an immutable map (e.g., Map.of(...))
instead of a mutable HashMap to prevent accidental mutation by callers.
In `@src/main/resources/static/app.js`:
- Around line 224-235: The remove-class → void flowDetail.offsetWidth →
add-class pattern around flowDetail (and the analogous rolePanel code) currently
has no effect because .flow-detail and .role-panel lack any CSS
transition/animation keyed to the .is-active class; either remove the DOM
class-toggle + forced reflow (delete the flowDetail.classList.remove / void
flowDetail.offsetWidth / flowDetail.classList.add lines) to simplify the code,
or add CSS animations for .flow-detail.is-active and .role-panel.is-active
(e.g., add transition + transform/opacity rules for .flow-detail and
.role-panel, and a different state for .is-active) so the existing flowDetail
and rolePanel class toggles actually trigger a visible fade/translate animation.
Ensure you update both the JS references (flowDetail, rolePanel) and the CSS
selectors (.flow-detail.is-active, .role-panel.is-active) consistently.
🪄 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: CHILL
Plan: Pro
Run ID: acaca7db-8400-475c-bbef-523a5886c30e
📒 Files selected for processing (5)
src/main/java/org/example/projektarendehantering/application/service/PublicStatsService.javasrc/main/java/org/example/projektarendehantering/presentation/web/UiController.javasrc/main/resources/static/app.csssrc/main/resources/static/app.jssrc/main/resources/templates/landing.html
Comment on lines
+1088
to
+1115
| /* Metric Chips */ | ||
| .metrics-row--hero { | ||
| justify-content: center; | ||
| gap: 20px; | ||
| margin-bottom: 48px; | ||
| } | ||
|
|
||
| .button-secondary { | ||
| border-color: rgba(231, 236, 255, 0.32); | ||
| background: rgba(255, 255, 255, 0.08); | ||
| .metric-chip { | ||
| background: rgba(255, 255, 255, 0.04); | ||
| border: 1px solid rgba(110, 168, 255, 0.15); | ||
| padding: 12px 20px; | ||
| border-radius: 14px; | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 14px; | ||
| transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1); | ||
| } | ||
|
|
||
| .button-secondary:hover { | ||
| background: rgba(255, 255, 255, 0.16); | ||
| .metric-chip:hover { | ||
| background: rgba(110, 168, 255, 0.08); | ||
| border-color: rgba(110, 168, 255, 0.4); | ||
| transform: translateY(-4px); | ||
| box-shadow: 0 10px 20px rgba(0, 0, 0, 0.2); | ||
| } | ||
|
|
||
| .button-strong { | ||
| border-color: rgba(180, 214, 255, 0.95); | ||
| background: linear-gradient(180deg, rgba(110, 168, 255, 0.6), rgba(93, 154, 251, 0.42)); | ||
| box-shadow: 0 0 0 1px rgba(180, 214, 255, 0.24), 0 10px 24px rgba(72, 142, 255, 0.34); | ||
| } | ||
| .metric-icon { font-size: 24px; } | ||
| .metric-content strong { font-size: 20px; color: #fff; display: block; } | ||
| .metric-content span { font-size: 12px; color: rgba(231, 236, 255, 0.6); text-transform: uppercase; letter-spacing: 0.05em; font-weight: 700; } |
There was a problem hiding this comment.
.metric-chip is redefined here and unintentionally restyles non-hero usages.
The original .metric-chip (line 928) is a small pill (min-height: 28px, border-radius: 999px, padding: 0 10px, font-size: 12px). This new definition replaces it everywhere with a much larger card-like element (padding: 12px 20px, border-radius: 14px, display: flex with gap: 14px, hover lift). However .metric-chip is reused as a plain text chip in several places that don't have the new .metric-icon/.metric-content child structure:
landing.htmllines 111–113 (Documents row: "MIME-kontroll", "Automatisk retry", "Säker uppföljning")landing.htmllines 129–131 (Audit row: "Aktör", "Händelse", "Tidpunkt")app.jsline 232 inrenderFlow(chips inside flow detail)app.jslines 278–279 inrenderRole(permission chips, including.permission-chip--allow/--deny)
All of these now render as oversized rounded rectangles rather than small pills, which contradicts the surrounding visual design and bloats those sections. Consider scoping the new style to a hero-only modifier instead of overriding the base class.
♻️ Suggested fix — scope new chip style with a modifier
-/* Metric Chips */
-.metrics-row--hero {
- justify-content: center;
- gap: 20px;
- margin-bottom: 48px;
-}
-
-.metric-chip {
- background: rgba(255, 255, 255, 0.04);
- border: 1px solid rgba(110, 168, 255, 0.15);
- padding: 12px 20px;
- border-radius: 14px;
- display: flex;
- align-items: center;
- gap: 14px;
- transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1);
-}
-
-.metric-chip:hover {
- background: rgba(110, 168, 255, 0.08);
- border-color: rgba(110, 168, 255, 0.4);
- transform: translateY(-4px);
- box-shadow: 0 10px 20px rgba(0, 0, 0, 0.2);
-}
+/* Metric Chips — hero variant only */
+.metrics-row--hero {
+ justify-content: center;
+ gap: 20px;
+ margin-bottom: 48px;
+}
+
+.metrics-row--hero .metric-chip {
+ background: rgba(255, 255, 255, 0.04);
+ border: 1px solid rgba(110, 168, 255, 0.15);
+ padding: 12px 20px;
+ border-radius: 14px;
+ display: flex;
+ align-items: center;
+ gap: 14px;
+ transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1);
+}
+
+.metrics-row--hero .metric-chip:hover {
+ background: rgba(110, 168, 255, 0.08);
+ border-color: rgba(110, 168, 255, 0.4);
+ transform: translateY(-4px);
+ box-shadow: 0 10px 20px rgba(0, 0, 0, 0.2);
+}📝 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
| /* Metric Chips */ | |
| .metrics-row--hero { | |
| justify-content: center; | |
| gap: 20px; | |
| margin-bottom: 48px; | |
| } | |
| .button-secondary { | |
| border-color: rgba(231, 236, 255, 0.32); | |
| background: rgba(255, 255, 255, 0.08); | |
| .metric-chip { | |
| background: rgba(255, 255, 255, 0.04); | |
| border: 1px solid rgba(110, 168, 255, 0.15); | |
| padding: 12px 20px; | |
| border-radius: 14px; | |
| display: flex; | |
| align-items: center; | |
| gap: 14px; | |
| transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1); | |
| } | |
| .button-secondary:hover { | |
| background: rgba(255, 255, 255, 0.16); | |
| .metric-chip:hover { | |
| background: rgba(110, 168, 255, 0.08); | |
| border-color: rgba(110, 168, 255, 0.4); | |
| transform: translateY(-4px); | |
| box-shadow: 0 10px 20px rgba(0, 0, 0, 0.2); | |
| } | |
| .button-strong { | |
| border-color: rgba(180, 214, 255, 0.95); | |
| background: linear-gradient(180deg, rgba(110, 168, 255, 0.6), rgba(93, 154, 251, 0.42)); | |
| box-shadow: 0 0 0 1px rgba(180, 214, 255, 0.24), 0 10px 24px rgba(72, 142, 255, 0.34); | |
| } | |
| .metric-icon { font-size: 24px; } | |
| .metric-content strong { font-size: 20px; color: #fff; display: block; } | |
| .metric-content span { font-size: 12px; color: rgba(231, 236, 255, 0.6); text-transform: uppercase; letter-spacing: 0.05em; font-weight: 700; } | |
| /* Metric Chips — hero variant only */ | |
| .metrics-row--hero { | |
| justify-content: center; | |
| gap: 20px; | |
| margin-bottom: 48px; | |
| } | |
| .metrics-row--hero .metric-chip { | |
| background: rgba(255, 255, 255, 0.04); | |
| border: 1px solid rgba(110, 168, 255, 0.15); | |
| padding: 12px 20px; | |
| border-radius: 14px; | |
| display: flex; | |
| align-items: center; | |
| gap: 14px; | |
| transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1); | |
| } | |
| .metrics-row--hero .metric-chip:hover { | |
| background: rgba(110, 168, 255, 0.08); | |
| border-color: rgba(110, 168, 255, 0.4); | |
| transform: translateY(-4px); | |
| box-shadow: 0 10px 20px rgba(0, 0, 0, 0.2); | |
| } | |
| .metric-icon { font-size: 24px; } | |
| .metric-content strong { font-size: 20px; color: `#fff`; display: block; } | |
| .metric-content span { font-size: 12px; color: rgba(231, 236, 255, 0.6); text-transform: uppercase; letter-spacing: 0.05em; font-weight: 700; } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/static/app.css` around lines 1088 - 1115, The new
large-card styles unintentionally override the base .metric-chip used across the
site; scope the hero-only styling instead of replacing .metric-chip by either
(A) prefixing the rules with .metrics-row--hero .metric-chip or (B) creating a
modifier like .metric-chip--hero and moving the large-card rules into that
selector (and updating .metric-icon and .metric-content selectors to
.metric-chip--hero .metric-icon / .metric-chip--hero .metric-content). Restore
the original small-pill properties on .metric-chip (min-height: 28px;
border-radius: 999px; padding: 0 10px; font-size: 12px) so non-hero chips keep
their intended appearance, and update any hero markup to use the new modifier if
you choose option B.
Comment on lines
+19
to
+41
| <div class="metrics-row metrics-row--hero"> | ||
| <div class="metric-chip"> | ||
| <span class="metric-icon">📋</span> | ||
| <div class="metric-content"> | ||
| <strong th:text="${stats.totalCases}">0</strong> | ||
| <span>Aktiva ärenden</span> | ||
| </div> | ||
| </div> | ||
| <div class="metric-chip"> | ||
| <span class="metric-icon">👥</span> | ||
| <div class="metric-content"> | ||
| <strong th:text="${stats.totalPatients}">0</strong> | ||
| <span>Patienter</span> | ||
| </div> | ||
| </div> | ||
| <div class="metric-chip"> | ||
| <span class="metric-icon">🩺</span> | ||
| <div class="metric-content"> | ||
| <strong th:text="${stats.totalEmployees}">0</strong> | ||
| <span>Vårdpersonal</span> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
The 0 inside the tag isn't a runtime fallback.
<strong th:text="${stats.totalCases}">0</strong> — the literal 0 is only rendered when Thymeleaf doesn't process the template (static preview). At runtime, if stats is missing or a key is absent, you'll get an empty string or null, not 0. To make this a real fallback, use the Elvis operator.
🛡️ Suggested fix
- <strong th:text="${stats.totalCases}">0</strong>
+ <strong th:text="${stats.totalCases ?: 0}">0</strong>
@@
- <strong th:text="${stats.totalPatients}">0</strong>
+ <strong th:text="${stats.totalPatients ?: 0}">0</strong>
@@
- <strong th:text="${stats.totalEmployees}">0</strong>
+ <strong th:text="${stats.totalEmployees ?: 0}">0</strong>📝 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
| <div class="metrics-row metrics-row--hero"> | |
| <div class="metric-chip"> | |
| <span class="metric-icon">📋</span> | |
| <div class="metric-content"> | |
| <strong th:text="${stats.totalCases}">0</strong> | |
| <span>Aktiva ärenden</span> | |
| </div> | |
| </div> | |
| <div class="metric-chip"> | |
| <span class="metric-icon">👥</span> | |
| <div class="metric-content"> | |
| <strong th:text="${stats.totalPatients}">0</strong> | |
| <span>Patienter</span> | |
| </div> | |
| </div> | |
| <div class="metric-chip"> | |
| <span class="metric-icon">🩺</span> | |
| <div class="metric-content"> | |
| <strong th:text="${stats.totalEmployees}">0</strong> | |
| <span>Vårdpersonal</span> | |
| </div> | |
| </div> | |
| </div> | |
| <div class="metrics-row metrics-row--hero"> | |
| <div class="metric-chip"> | |
| <span class="metric-icon">📋</span> | |
| <div class="metric-content"> | |
| <strong th:text="${stats.totalCases ?: 0}">0</strong> | |
| <span>Aktiva ärenden</span> | |
| </div> | |
| </div> | |
| <div class="metric-chip"> | |
| <span class="metric-icon">👥</span> | |
| <div class="metric-content"> | |
| <strong th:text="${stats.totalPatients ?: 0}">0</strong> | |
| <span>Patienter</span> | |
| </div> | |
| </div> | |
| <div class="metric-chip"> | |
| <span class="metric-icon">🩺</span> | |
| <div class="metric-content"> | |
| <strong th:text="${stats.totalEmployees ?: 0}">0</strong> | |
| <span>Vårdpersonal</span> | |
| </div> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/templates/landing.html` around lines 19 - 41, The template
currently uses literal "0" inside the <strong> tags as a static preview, but
that does not serve as a runtime fallback; update the three <strong> elements
that use th:text (the ones bound to stats.totalCases, stats.totalPatients, and
stats.totalEmployees) to use the Thymeleaf/OGNL Elvis operator in their th:text
expressions so that they render 0 when the stats value is null or missing
(replace the current th:text expressions on those <strong> elements with an
Elvis-style fallback producing 0).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
closes #109
Summary by CodeRabbit
New Features
Style
Improvements