feat: add details modals for users, clinics and audit logs#264
Merged
johanbriger merged 5 commits intoApr 24, 2026
Hidden character warning
The head ref may contain hidden characters: "247-kunna-\u00f6ppna-och-se-detaljer-f\u00f6r-anv\u00e4ndare-under-admin"
Merged
Conversation
📝 WalkthroughWalkthroughThree new admin detail modals (User, Clinic, Log) are added; AuditLogView integrates LogDetailsModal and refines list/filter UI; AdminDashboard adds state/handlers to open details modals and bridge from details to edit flows, and clinic save now awaits data refresh. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as "Admin UI\n(AdminDashboard)"
participant API as "Backend API"
participant UserModal as "UserDetailsModal"
participant VetSvc as "vetService"
rect rgba(200,220,255,0.5)
Admin->>Admin: click user row
end
rect rgba(200,255,200,0.5)
Admin->>UserModal: open(user)
UserModal-->>Admin: render (isOpen,user)
end
rect rgba(255,220,200,0.5)
UserModal->>VetSvc: getAll() (if role includes VET)
VetSvc-->>UserModal: vets[]
UserModal->>Admin: show vet info / enable Edit
end
rect rgba(240,240,240,0.5)
Admin->>API: save clinic/user (edit flow)
API-->>Admin: response
Admin->>Admin: fetchData() (refresh)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 8
♻️ Duplicate comments (1)
frontend/src/components/admin/UserDetailsModal.jsx (1)
122-126:⚠️ Potential issue | 🟡 MinorID truncation always appends "…" (same nit as
ClinicDetailsModal).If
user.idis shorter than 13 chars, the ellipsis is still shown. Only append when truncated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/UserDetailsModal.jsx` around lines 122 - 126, The displayed User ID always appends "..." even when not truncated; update the rendering in UserDetailsModal (the JSX referencing user.id?.substring(0, 13)) to conditionally append the ellipsis only when the ID length exceeds 13 (e.g., compute a displayId like user.id ? (user.id.length > 13 ? user.id.substring(0,13) + '...' : user.id) : '' and render that), ensuring you use the existing user.id reference in the component.
🧹 Nitpick comments (4)
frontend/src/components/admin/UserDetailsModal.jsx (2)
31-42: Same dialog/keyboard a11y gaps asClinicDetailsModal.No
role="dialog",aria-modal, Escape handler, or backdrop click-to-close. Worth fixing in both modals together, e.g. by extracting a shared<Modal>wrapper that handles focus trap, ESC, and backdrop dismissal so the three new modals (user/clinic/log) stay consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/UserDetailsModal.jsx` around lines 31 - 42, UserDetailsModal (and ClinicDetailsModal) lack accessible dialog semantics and behaviors—add a shared Modal wrapper component to centralize role="dialog" and aria-modal="true", focus trapping, Escape-key handling, and backdrop click-to-close; implement a Modal component that renders the existing modal container as its child, sets role="dialog" aria-modal="true" on the dialog element, captures Escape key to call the passed onClose prop, closes when clicking the backdrop (but not when clicking inside the dialog), and enforces a focus trap (restore focus on close); then replace the modal markup in UserDetailsModal.jsx and ClinicDetailsModal with usage of this Modal wrapper (and reuse for the new Logs modal) so all three get consistent a11y and keyboard behavior.
13-17: ReplacevetService.getAll()withvetService.getById(user.id).The current implementation downloads the entire vet roster each time an admin views a VET user's details. The backend already supports a by-user endpoint:
GET /api/vets/{userId}(viavetService.getById()). Use it directly instead of fetching all vets and filtering client-side.Before
vetService.getAll() .then(res => { const details = res.data.find(v => v.userId === user.id); setVetDetails(details); })After
vetService.getById(user.id) .then(res => { setVetDetails(res.data); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/UserDetailsModal.jsx` around lines 13 - 17, Replace the client-side filtering of the full vet list by calling the dedicated endpoint: change the vetService.getAll() usage to vetService.getById(user.id) and set vet details directly from the response (use setVetDetails with res.data). Locate the block where vetService.getAll() is used (the .then that finds by v.userId === user.id) and update it to call vetService.getById(user.id) and assign the returned payload to setVetDetails.frontend/src/pages/AdminDashboard.jsx (2)
29-33:selectedItemis shared across User and Clinic flows.A single
selectedItemcarries both user and clinic entities depending on the active tab. Today the flows don't interleave, but any future feature that opens a user-details view while a clinic modal is still mounted (or vice versa) will read the wrong type. Consider splitting intoselectedUser/selectedClinicto make the coupling explicit and future‑proof.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminDashboard.jsx` around lines 29 - 33, The code uses a single shared state variable selectedItem (and setSelectedItem) for both user and clinic flows which can cause type confusion; replace this with two explicit states selectedUser / setSelectedUser and selectedClinic / setSelectedClinic and update all places that read/write selectedItem (including modal open handlers and components that rely on isUserModalOpen, isClinicModalOpen, isDetailsModalOpen, isClinicDetailsOpen) so user flows use selectedUser and clinic flows use selectedClinic, and ensure modal open/close logic sets/clears the appropriate new state instead of selectedItem.
128-143: FragilesetTimeout(100)bridge between details and edit modals.Both handlers use a 100 ms delay to sequence state changes, which is timing‑dependent (can still produce a flicker on slow devices or miss entirely on fast ones) and duplicates logic. Since React will batch these state updates within the same tick anyway, you can drop the timeout and extract a single helper.
♻️ Proposed consolidation
- // Sömlös redigering från detaljvyer - const handleEditFromDetails = (user) => { - setIsDetailsModalOpen(false); - setTimeout(() => { - setSelectedItem(user); - setIsUserModalOpen(true); - }, 100); - }; - - const handleEditClinicFromDetails = (clinic) => { - setIsClinicDetailsOpen(false); - setTimeout(() => { - setSelectedItem(clinic); - setIsClinicModalOpen(true); - }, 100); - }; + // Sömlös redigering från detaljvyer + const bridgeToEdit = (item, closeDetails, openEdit) => { + closeDetails(false); + setSelectedItem(item); + openEdit(true); + }; + const handleEditFromDetails = (user) => + bridgeToEdit(user, setIsDetailsModalOpen, setIsUserModalOpen); + const handleEditClinicFromDetails = (clinic) => + bridgeToEdit(clinic, setIsClinicDetailsOpen, setIsClinicModalOpen);If a brief unmount is desired for the transition, prefer CSS/transition hooks over a magic delay.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminDashboard.jsx` around lines 128 - 143, Both handlers (handleEditFromDetails and handleEditClinicFromDetails) use a fragile setTimeout(100) bridge and duplicate logic; remove the timeout and consolidate the common behavior into a helper to avoid timing issues. Implement a single helper (e.g., openEditModal or openEditFromDetails) that accepts the item and the modal setter (or a modal type) and does: setSelectedItem(item); call the appropriate modal open setter (setIsUserModalOpen or setIsClinicModalOpen); and replace both handleEditFromDetails and handleEditClinicFromDetails to first close their detail modal (setIsDetailsModalOpen(false) or setIsClinicDetailsOpen(false)) and then call the helper immediately (no setTimeout). If you need a brief unmount for animations, handle that with CSS/transition hooks instead of a hard-coded timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/AuditLogView.jsx`:
- Around line 236-240: The row div that calls openLogDetails(log) is not
keyboard-accessible; change it so keyboard and AT users can activate it by
either replacing the div with a <button type="button"> that keeps the same
className/styling, or add accessibility attributes and handlers to the existing
div: add role="button", tabIndex={0}, and an onKeyDown handler that calls
openLogDetails(log) when Enter or Space is pressed (and include an appropriate
aria-label or ensure accessible text). Update the element where openLogDetails
is referenced to implement one of these fixes so the row is reachable and
activatable by keyboard.
- Around line 19-114: This file defines a duplicate LogDetailsModal component
that conflicts with the new standalone module; delete the entire local
LogDetailsModal declaration and instead import and use the exported
LogDetailsModal component (ensure you reference the same component name where
it's rendered), remove any stray local references (props like isOpen, onClose,
log) only if already satisfied by the imported component, and ensure you do not
export another LogDetailsModal from this file so there is a single source of
truth.
In `@frontend/src/components/admin/ClinicDetailsModal.jsx`:
- Around line 113-116: The ClinicDetailsModal currently always appends "..."
after rendering clinic.id?.substring(0, 13) which causes ellipsis even when the
ID is shorter than 13; update the rendering in ClinicDetailsModal.jsx to check
clinic.id length (or existence) and only append "..." when clinic.id.length > 13
(i.e., truncation occurred), otherwise render the full clinic.id as-is;
reference the clinic.id usage in the JSX where substring(0, 13) is used to
locate and change the logic.
- Around line 22-46: Add proper dialog semantics and keyboard affordances to the
ClinicDetailsModal component: give the modal container element role="dialog" and
aria-modal="true" and set aria-labelledby to an id you add to the <h2> (e.g.,
id="clinic-details-title"); wire an Escape key handler using useEffect that
listens for "Escape" while isOpen is true and calls onClose; and make the
backdrop clickable so clicks outside the modal content invoke onClose (ensure
clicks inside the inner dialog don’t propagate). Reference the
ClinicDetailsModal component, the existing onClose prop, the <h2> heading, and
useEffect/isOpen to locate where to add these changes.
In `@frontend/src/components/admin/LogDetailsModal.jsx`:
- Line 45: The display uses log.action.replace('_', ' ') which only replaces the
first underscore and will throw if log.action is undefined; update the rendering
in LogDetailsModal (the expression that references log.action) to safely handle
missing actions with optional chaining or a fallback and replace all underscores
(e.g., use a global replace or split/join) so multi-underscore values like
USER_ROLE_UPDATED become "USER ROLE UPDATED" without throwing when log.action is
absent.
- Around line 26-132: Add keyboard, backdrop, and accessibility semantics to the
modal: attach a useEffect in LogDetailsModal.jsx that registers a keydown
listener for "Escape" which calls the existing onClose and cleans up on unmount;
make the outer backdrop div clickable (e.g., onClick or onMouseDown) and only
call onClose when the event target is the backdrop container (to short-circuit
inner clicks) instead of always closing; add role="dialog" aria-modal="true" and
aria-labelledby pointing to the modal title (give the <h2> a stable id like
"log-title") on the modal container so assistive tech recognizes it; ensure you
reference the existing onClose handler and the <h2> title element when wiring
these changes.
In `@frontend/src/components/admin/UserDetailsModal.jsx`:
- Line 5: The UserDetailsModal component should default its clinics prop to an
empty array and avoid assuming clinics is defined: change the component
signature (UserDetailsModal) to set clinics = [] so clinics.find(...) is safe,
and replace the loading-sounding fallback 'Laddar…' used with userClinic?.name
with a neutral fallback like 'Okänd klinik' to reflect a missing clinic rather
than a loading state; ensure any usage of clinics.find in UserDetailsModal
checks the array via the default instead of relying on callers to pass clinics.
- Around line 9-23: The vetService.getAll() call in the useEffect (watching
isOpen and user) can resolve after a newer fetch and overwrite state; add a
cancellation/ignore mechanism in that effect: create an "isMounted" or "ignore"
flag (or use AbortController) scoped to the effect, check it before calling
setVetDetails and setLoadingVet in then/catch/finally, and set the flag/abort in
the effect cleanup; ensure the logic that sets vet details only runs when isOpen
&& user?.role includes 'VET' and the request is still valid.
---
Duplicate comments:
In `@frontend/src/components/admin/UserDetailsModal.jsx`:
- Around line 122-126: The displayed User ID always appends "..." even when not
truncated; update the rendering in UserDetailsModal (the JSX referencing
user.id?.substring(0, 13)) to conditionally append the ellipsis only when the ID
length exceeds 13 (e.g., compute a displayId like user.id ? (user.id.length > 13
? user.id.substring(0,13) + '...' : user.id) : '' and render that), ensuring you
use the existing user.id reference in the component.
---
Nitpick comments:
In `@frontend/src/components/admin/UserDetailsModal.jsx`:
- Around line 31-42: UserDetailsModal (and ClinicDetailsModal) lack accessible
dialog semantics and behaviors—add a shared Modal wrapper component to
centralize role="dialog" and aria-modal="true", focus trapping, Escape-key
handling, and backdrop click-to-close; implement a Modal component that renders
the existing modal container as its child, sets role="dialog" aria-modal="true"
on the dialog element, captures Escape key to call the passed onClose prop,
closes when clicking the backdrop (but not when clicking inside the dialog), and
enforces a focus trap (restore focus on close); then replace the modal markup in
UserDetailsModal.jsx and ClinicDetailsModal with usage of this Modal wrapper
(and reuse for the new Logs modal) so all three get consistent a11y and keyboard
behavior.
- Around line 13-17: Replace the client-side filtering of the full vet list by
calling the dedicated endpoint: change the vetService.getAll() usage to
vetService.getById(user.id) and set vet details directly from the response (use
setVetDetails with res.data). Locate the block where vetService.getAll() is used
(the .then that finds by v.userId === user.id) and update it to call
vetService.getById(user.id) and assign the returned payload to setVetDetails.
In `@frontend/src/pages/AdminDashboard.jsx`:
- Around line 29-33: The code uses a single shared state variable selectedItem
(and setSelectedItem) for both user and clinic flows which can cause type
confusion; replace this with two explicit states selectedUser / setSelectedUser
and selectedClinic / setSelectedClinic and update all places that read/write
selectedItem (including modal open handlers and components that rely on
isUserModalOpen, isClinicModalOpen, isDetailsModalOpen, isClinicDetailsOpen) so
user flows use selectedUser and clinic flows use selectedClinic, and ensure
modal open/close logic sets/clears the appropriate new state instead of
selectedItem.
- Around line 128-143: Both handlers (handleEditFromDetails and
handleEditClinicFromDetails) use a fragile setTimeout(100) bridge and duplicate
logic; remove the timeout and consolidate the common behavior into a helper to
avoid timing issues. Implement a single helper (e.g., openEditModal or
openEditFromDetails) that accepts the item and the modal setter (or a modal
type) and does: setSelectedItem(item); call the appropriate modal open setter
(setIsUserModalOpen or setIsClinicModalOpen); and replace both
handleEditFromDetails and handleEditClinicFromDetails to first close their
detail modal (setIsDetailsModalOpen(false) or setIsClinicDetailsOpen(false)) and
then call the helper immediately (no setTimeout). If you need a brief unmount
for animations, handle that with CSS/transition hooks instead of a hard-coded
timeout.
🪄 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: a877155e-8fdf-431f-baea-91627030b591
📒 Files selected for processing (5)
frontend/src/components/admin/AuditLogView.jsxfrontend/src/components/admin/ClinicDetailsModal.jsxfrontend/src/components/admin/LogDetailsModal.jsxfrontend/src/components/admin/UserDetailsModal.jsxfrontend/src/pages/AdminDashboard.jsx
Comment on lines
+236
to
+240
| <div | ||
| key={log.id} | ||
| onClick={() => openLogDetails(log)} | ||
| className="relative flex gap-6 group border-b border-slate-50 last:border-0 cursor-pointer hover:bg-slate-50/80 p-2 rounded-2xl transition-all" | ||
| > |
There was a problem hiding this comment.
Clickable row is not keyboard-accessible.
Opening log details requires a mouse — the <div> has onClick but no role="button", tabIndex={0}, or onKeyDown handler, so users relying on keyboard/AT can’t reach the details view. Either add those attributes or render the row as a <button type="button"> with the current styling.
♻️ Proposed fix
<div
key={log.id}
+ role="button"
+ tabIndex={0}
onClick={() => openLogDetails(log)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ openLogDetails(log);
+ }
+ }}
className="relative flex gap-6 group border-b border-slate-50 last:border-0 cursor-pointer hover:bg-slate-50/80 p-2 rounded-2xl transition-all"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/AuditLogView.jsx` around lines 236 - 240, The
row div that calls openLogDetails(log) is not keyboard-accessible; change it so
keyboard and AT users can activate it by either replacing the div with a <button
type="button"> that keeps the same className/styling, or add accessibility
attributes and handlers to the existing div: add role="button", tabIndex={0},
and an onKeyDown handler that calls openLogDetails(log) when Enter or Space is
pressed (and include an appropriate aria-label or ensure accessible text).
Update the element where openLogDetails is referenced to implement one of these
fixes so the row is reachable and activatable by keyboard.
Comment on lines
+113
to
+116
| <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic">Klinik-ID</p> | ||
| <p className="font-mono text-[10px] text-slate-400"> | ||
| {clinic.id?.substring(0, 13)}... | ||
| </p> |
There was a problem hiding this comment.
Ellipsis is appended even when the ID is shorter than 13 chars.
clinic.id?.substring(0, 13) + '...' will always render ... even if the full ID is returned. Only append the ellipsis when truncation actually happened.
💡 Proposed fix
- <p className="font-mono text-[10px] text-slate-400">
- {clinic.id?.substring(0, 13)}...
- </p>
+ <p className="font-mono text-[10px] text-slate-400">
+ {clinic.id
+ ? clinic.id.length > 13 ? `${clinic.id.substring(0, 13)}…` : clinic.id
+ : '---'}
+ </p>📝 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
| <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic">Klinik-ID</p> | |
| <p className="font-mono text-[10px] text-slate-400"> | |
| {clinic.id?.substring(0, 13)}... | |
| </p> | |
| <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic">Klinik-ID</p> | |
| <p className="font-mono text-[10px] text-slate-400"> | |
| {clinic.id | |
| ? clinic.id.length > 13 ? `${clinic.id.substring(0, 13)}…` : clinic.id | |
| : '---'} | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/ClinicDetailsModal.jsx` around lines 113 - 116,
The ClinicDetailsModal currently always appends "..." after rendering
clinic.id?.substring(0, 13) which causes ellipsis even when the ID is shorter
than 13; update the rendering in ClinicDetailsModal.jsx to check clinic.id
length (or existence) and only append "..." when clinic.id.length > 13 (i.e.,
truncation occurred), otherwise render the full clinic.id as-is; reference the
clinic.id usage in the JSX where substring(0, 13) is used to locate and change
the logic.
| <div> | ||
| <p className="text-[10px] font-black text-purple-400 uppercase tracking-widest italic mb-1">Systemlogg / Händelse</p> | ||
| <h2 className="text-2xl font-black text-slate-900 italic tracking-tight uppercase leading-none"> | ||
| {log.action.replace('_', ' ')} |
There was a problem hiding this comment.
replace('_', ' ') only replaces the first underscore.
Current audit actions (CASE_CREATED, STATUS_CHANGED, etc.) all have a single underscore so this works today, but the moment a multi-underscore action appears (e.g. USER_ROLE_UPDATED) it will render as USER ROLE_UPDATED. Also, log.action is not optional-chained, so an absent action will throw when the modal opens.
♻️ Proposed fix
- {log.action.replace('_', ' ')}
+ {log.action?.replaceAll('_', ' ')}📝 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
| {log.action.replace('_', ' ')} | |
| {log.action?.replaceAll('_', ' ')} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/LogDetailsModal.jsx` at line 45, The display
uses log.action.replace('_', ' ') which only replaces the first underscore and
will throw if log.action is undefined; update the rendering in LogDetailsModal
(the expression that references log.action) to safely handle missing actions
with optional chaining or a fallback and replace all underscores (e.g., use a
global replace or split/join) so multi-underscore values like USER_ROLE_UPDATED
become "USER ROLE UPDATED" without throwing when log.action is absent.
| import { X, Mail, Shield, Hospital, Calendar, Activity, User as UserIcon, Edit2, Award, Briefcase } from 'lucide-react'; | ||
| import { vetService } from '../../services/api'; | ||
|
|
||
| const UserDetailsModal = ({ isOpen, onClose, user, clinics, onEdit }) => { |
There was a problem hiding this comment.
Default clinics to [] and reconsider the "Laddar…" fallback.
clinics.find(...) on Line 27 will throw TypeError if clinics is ever omitted (e.g. tests, or a future caller). ClinicDetailsModal already defaults users = []; keep the API consistent. Additionally, userClinic?.name || 'Laddar...' on Line 81 implies loading state even when the clinic genuinely isn't in the list — consider a neutral fallback like 'Okänd klinik'.
💡 Proposed fix
-const UserDetailsModal = ({ isOpen, onClose, user, clinics, onEdit }) => {
+const UserDetailsModal = ({ isOpen, onClose, user, clinics = [], onEdit }) => {- {userClinic?.name || 'Laddar...'}
+ {userClinic?.name || 'Okänd klinik'}Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/UserDetailsModal.jsx` at line 5, The
UserDetailsModal component should default its clinics prop to an empty array and
avoid assuming clinics is defined: change the component signature
(UserDetailsModal) to set clinics = [] so clinics.find(...) is safe, and replace
the loading-sounding fallback 'Laddar…' used with userClinic?.name with a
neutral fallback like 'Okänd klinik' to reflect a missing clinic rather than a
loading state; ensure any usage of clinics.find in UserDetailsModal checks the
array via the default instead of relying on callers to pass clinics.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/components/admin/ClinicDetailsModal.jsx (1)
16-30: Consider guarding againstonCloseidentity churn and tightening the effect.Because
onCloseis in the dependency array andAdminDashboardpasses an inline arrow (onClose={() => setIsClinicDetailsOpen(false)}), this effect tears down and re-attaches thekeydownlistener on every parent render while the modal is open. Functionally fine, but it also means thedocument.body.style.overflow = 'unset'cleanup runs on every re-render — if any ancestor temporarily setsoverflowelsewhere, the churn could race with it.Two low-risk improvements:
- Early-return from the effect when
!isOpenso the cleanup only runs on actual open→close transitions.- Either memoize
onCloseat the call site or read it from a ref inside the handler so the listener doesn't need to be re-bound.♻️ Proposed refactor
- useEffect(() => { - const handleEscape = (e) => { - if (e.key === 'Escape') onClose(); - }; - - if (isOpen) { - window.addEventListener('keydown', handleEscape); - document.body.style.overflow = 'hidden'; // Lås bakgrundsscroll - } - - return () => { - window.removeEventListener('keydown', handleEscape); - document.body.style.overflow = 'unset'; // Återställ scroll - }; - }, [isOpen, onClose]); + useEffect(() => { + if (!isOpen) return; + + const handleEscape = (e) => { + if (e.key === 'Escape') onClose(); + }; + + window.addEventListener('keydown', handleEscape); + const previousOverflow = document.body.style.overflow; + document.body.style.overflow = 'hidden'; + + return () => { + window.removeEventListener('keydown', handleEscape); + document.body.style.overflow = previousOverflow; + }; + }, [isOpen, onClose]);Note: preserving the previous
overflowvalue instead of hard-coding'unset'is also safer if some other component (or a global style) has already set it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/ClinicDetailsModal.jsx` around lines 16 - 30, The effect in useEffect with handleEscape and document.body.style currently rebinds on every render because onClose is in the deps (AdminDashboard passes an inline arrow), so change the effect to early-return when !isOpen, capture and restore the previous document.body.style.overflow value, and avoid rebinding the listener by reading the latest onClose from a ref inside handleEscape (or instruct the caller to memoize onClose); update references to useEffect, handleEscape, isOpen, onClose and document.body.style.overflow accordingly so the keydown listener is only added/removed on open/close transitions and overflow is restored safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/ClinicDetailsModal.jsx`:
- Line 105: The className on the paragraph in ClinicDetailsModal.jsx contains a
duplicated "italic" token; locate the <p> element rendering "Anslutna
Veterinärer ({clinicStaff.length})" and remove the extra "italic" so the
className only includes a single "italic" entry (keep all other classes
unchanged).
- Around line 154-160: The button click currently calls the onEdit prop directly
which throws if it's missing; update ClinicDetailsModal to guard onEdit by
either providing a default no-op (e.g., defaultProps or default parameter) or
conditionally render the button only when typeof onEdit === 'function' so
clicking won't cause TypeError; ensure you update the component definition that
references onEdit and the JSX button handler to use the chosen guard.
---
Nitpick comments:
In `@frontend/src/components/admin/ClinicDetailsModal.jsx`:
- Around line 16-30: The effect in useEffect with handleEscape and
document.body.style currently rebinds on every render because onClose is in the
deps (AdminDashboard passes an inline arrow), so change the effect to
early-return when !isOpen, capture and restore the previous
document.body.style.overflow value, and avoid rebinding the listener by reading
the latest onClose from a ref inside handleEscape (or instruct the caller to
memoize onClose); update references to useEffect, handleEscape, isOpen, onClose
and document.body.style.overflow accordingly so the keydown listener is only
added/removed on open/close transitions and overflow is restored safely.
🪄 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: 6a1872ba-5e9f-4fc0-a607-8ac01cee72bb
📒 Files selected for processing (4)
frontend/src/components/admin/AuditLogView.jsxfrontend/src/components/admin/ClinicDetailsModal.jsxfrontend/src/components/admin/LogDetailsModal.jsxfrontend/src/components/admin/UserDetailsModal.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/admin/UserDetailsModal.jsx
- frontend/src/components/admin/LogDetailsModal.jsx
| <div className="pt-6 border-t border-slate-100"> | ||
| <div className="flex items-center gap-2 mb-4"> | ||
| <Award size={18} className="text-emerald-500" /> | ||
| <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic italic">Anslutna Veterinärer ({clinicStaff.length})</p> |
There was a problem hiding this comment.
Duplicate italic class.
italic italic is repeated in the className. Harmless but should be deduplicated.
🧹 Proposed fix
- <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic italic">Anslutna Veterinärer ({clinicStaff.length})</p>
+ <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic">Anslutna Veterinärer ({clinicStaff.length})</p>📝 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
| <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic italic">Anslutna Veterinärer ({clinicStaff.length})</p> | |
| <p className="text-[10px] font-black text-slate-400 uppercase tracking-widest italic">Anslutna Veterinärer ({clinicStaff.length})</p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/ClinicDetailsModal.jsx` at line 105, The
className on the paragraph in ClinicDetailsModal.jsx contains a duplicated
"italic" token; locate the <p> element rendering "Anslutna Veterinärer
({clinicStaff.length})" and remove the extra "italic" so the className only
includes a single "italic" entry (keep all other classes unchanged).
Comment on lines
+154
to
+160
| <button | ||
| onClick={() => onEdit(clinic)} | ||
| className="w-full flex items-center justify-center gap-2 py-4 bg-slate-900 text-white text-[10px] font-black uppercase tracking-[0.2em] rounded-2xl hover:bg-emerald-600 transition-all shadow-xl active:scale-95 italic" | ||
| > | ||
| <Edit2 size={14} /> | ||
| Redigera klinikuppgifter | ||
| </button> |
There was a problem hiding this comment.
Guard against a missing onEdit prop.
onEdit isn't defaulted like users, so if a caller ever mounts this modal without passing it, clicking "Redigera klinikuppgifter" throws TypeError: onEdit is not a function. Either default it to a no-op / hide the button when absent, or mark it required via PropTypes / TS.
🛡️ Minimal fix
-const ClinicDetailsModal = ({ isOpen, onClose, clinic, users = [], onEdit }) => {
+const ClinicDetailsModal = ({ isOpen, onClose, clinic, users = [], onEdit }) => {
...
- <button
- onClick={() => onEdit(clinic)}
+ <button
+ onClick={() => onEdit?.(clinic)}
+ disabled={!onEdit}
className="w-full flex items-center justify-center gap-2 py-4 bg-slate-900 text-white text-[10px] font-black uppercase tracking-[0.2em] rounded-2xl hover:bg-emerald-600 transition-all shadow-xl active:scale-95 italic"
>📝 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
| <button | |
| onClick={() => onEdit(clinic)} | |
| className="w-full flex items-center justify-center gap-2 py-4 bg-slate-900 text-white text-[10px] font-black uppercase tracking-[0.2em] rounded-2xl hover:bg-emerald-600 transition-all shadow-xl active:scale-95 italic" | |
| > | |
| <Edit2 size={14} /> | |
| Redigera klinikuppgifter | |
| </button> | |
| <button | |
| onClick={() => onEdit?.(clinic)} | |
| disabled={!onEdit} | |
| className="w-full flex items-center justify-center gap-2 py-4 bg-slate-900 text-white text-[10px] font-black uppercase tracking-[0.2em] rounded-2xl hover:bg-emerald-600 transition-all shadow-xl active:scale-95 italic" | |
| > | |
| <Edit2 size={14} /> | |
| Redigera klinikuppgifter | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/admin/ClinicDetailsModal.jsx` around lines 154 - 160,
The button click currently calls the onEdit prop directly which throws if it's
missing; update ClinicDetailsModal to guard onEdit by either providing a default
no-op (e.g., defaultProps or default parameter) or conditionally render the
button only when typeof onEdit === 'function' so clicking won't cause TypeError;
ensure you update the component definition that references onEdit and the JSX
button handler to use the chosen guard.
johanbriger
deleted the
247-kunna-öppna-och-se-detaljer-för-användare-under-admin
branch
April 24, 2026 07:47
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
1 participant
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 #247
Summary by CodeRabbit