Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a Gallery feature: backend upload controller and admin page, image resize/optimization utilities, new entity fields and settings; frontend adds gallery route, data fetching hook, UI components (masonry, image), config fields, types, and localization. Changes
Sequence DiagramssequenceDiagram
participant Admin as Administrator
participant Browser as Browser
participant Controller as GalleryUploadController
participant Util as Image Utilities
participant Storage as StorageService
participant DB as Database
Admin->>Browser: Select files + metadata
Browser->>Controller: POST /admin/control/gallery-upload (multipart)
Controller->>Controller: Validate auth & PERMISSION_CREATE_GALLERY
loop per file
Controller->>Storage: saveNamedObject(original image)
Storage-->>Controller: original URL
Controller->>Util: resizeImage(800,800)
Util-->>Controller: resized image bytes
Controller->>Util: optimizeImage(extension)
Util-->>Controller: optimized bytes
Controller->>Storage: saveNamedObject(thumbnail)
Storage-->>Controller: thumbnail URL
Controller->>DB: persist GalleryEntity (title, description, url, thumbnailUrl, flags)
DB-->>Controller: persisted
end
Controller->>Browser: Redirect with uploaded names
sequenceDiagram
participant User as User
participant App as React App
participant Config as Config Context
participant API as API Endpoint
User->>App: Navigate to /gallery
App->>Config: read gallery config (title, topMessage, bottomMessage)
App->>API: useGalleryQuery -> GET /api/gallery
API-->>App: GalleryView { photos[] }
App->>User: Render page with topMessage, GalleryMasonry(photos), bottomMessage
User->>GalleryMasonry: click thumbnail
GalleryMasonry->>App: open Dialog with full-size image
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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. ✨ 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 |
Isti01
commented
Apr 5, 2026
| return ( | ||
| <div className={cn('relative', className)}> | ||
| {!loaded && <Skeleton className="absolute inset-0 h-full w-full rounded-lg" />} | ||
| <img |
Collaborator
Author
There was a problem hiding this comment.
I don't like that this doesn't have loading="lazy" but the images weren't loading when I set it. Weird
Isti01
force-pushed
the
feature/gallery-frontend
branch
from
066d1a4 to
2cc9bf2
Compare
Isti01
force-pushed
the
feature/gallery-frontend
branch
from
2cc9bf2 to
e0d5806
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt (1)
127-127: Prefer flash attributes over query-string payload for upload feedback.Appending all uploaded names into the redirect URL can hit URL-size limits and leaks noisy data into logs/history.
RedirectAttributesis safer here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt` at line 127, The redirect currently appends uploadedPhotos into the query string (return "redirect:/admin/control/gallery-upload?uploaded=${uploadedPhotos.joinToString(",").urlEncode()}"), which can overflow URLs and leak data; modify the controller method in GalleryUploadController to accept a RedirectAttributes parameter (org.springframework.web.servlet.mvc.support.RedirectAttributes) and call redirectAttributes.addFlashAttribute("uploadedPhotos", uploadedPhotos) (or a concise DTO/list name) instead of encoding into the URL, then change the return to "redirect:/admin/control/gallery-upload" and remove the urlEncode/joinToString usage so the upload feedback is carried via flash attributes rather than the query string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt`:
- Around line 108-124: The code currently saves the original file and DB row
even if thumbnail generation or thumbnail storage fails; change the flow so you
generate the thumbnail first (call generateThumbnail(file, extension)) and
attempt to save the thumbnail with storageService.saveNamedObject("gallery",
thumbName, ...), and only if that returns a non-empty URL proceed to save the
original (storageService.saveNamedObject("gallery", fileName, file)) and then
create the GalleryEntity and call galleryService.savePhoto(entity); ensure you
do not create the GalleryEntity or call savePhoto when generateThumbnail returns
null or the thumbnail save returns empty (and if you must keep current order,
ensure you delete the original via the storage service when thumbnail save
fails).
- Around line 88-98: The upload logic in GalleryUploadController currently only
checks names.size against files.size, allowing descriptions, highlighted, or
showOnHomePage to be shorter/misaligned and silently produce wrong metadata;
update the validation to require all per-image lists (names, descriptions,
highlighted, showOnHomePage) either be equal to files.size or be entirely
absent/empty, and if not throw an IllegalArgumentException. Locate the method
using variables names, files, descriptions, highlighted, showOnHomePage (the
upload loop building uploadedPhotos) and add size checks for descriptions.size,
highlighted.size, and showOnHomePage.size (or normalize inputs up-front by
expanding/trimming to files.size) so metadata indices always align with files.
- Around line 108-123: Wrap the call to galleryService.savePhoto(entity) in a
try/catch and if it throws, remove any already-stored objects by calling
storageService.deleteObject("gallery", fileName) and, if a thumbnail was stored
(thumbUrl non-empty), storageService.deleteObject("gallery", thumbName) (or
storageService.delete equivalents) before rethrowing or converting to an
appropriate error response; ensure you reference the existing locals (fileName,
thumbName, url, thumbUrl), perform cleanup only when storage succeeded, and
avoid swallowing the original exception so callers still get the failure.
In `@backend/src/main/kotlin/hu/bme/sch/cmsch/util/Utility.kt`:
- Around line 126-132: The computed newWidth/newHeight can become 0 for extreme
aspect ratios before creating the BufferedImage; clamp the values after
calculating scalingFactor by ensuring newWidth and newHeight are at least 1
(e.g., replace the direct toInt results with max(1, (width *
scalingFactor).toInt()) and max(1, (height * scalingFactor).toInt())) so the
BufferedImage(...) call never receives zero dimensions; update the calculations
around scalingFactor, newWidth, newHeight to enforce this minimum.
- Around line 142-169: Validate the requested image format and ensure an
ImageWriter exists before attempting to write: check that the extracted
extension is one of the supported formats (e.g., "jpg","jpeg","png","webp" as
applicable) and reject/normalize unknown extensions early; when using
ImageIO.getImageWritersByFormatName("jpeg") do not call .next() blindly—verify
the iterator hasNext() and throw or return a clear error if no JPEG writer is
available; for the non-JPEG branch check the boolean result of
ImageIO.write(this, extension, baos) and handle a false return by failing with a
clear exception or falling back to a safe default writer/format (for example,
write as "jpeg" after converting transparency using the existing noAlpha logic);
refer to the variables and calls extension,
ImageIO.getImageWritersByFormatName("jpeg"), ImageIO.write(this, extension,
baos), JPEGImageWriteParam and noAlpha to locate where to add these checks and
error handling.
In `@backend/src/main/resources/templates/uploadGallery.html`:
- Line 150: The template includes an <img> element with class "image-preview"
that has an empty src which triggers HTML lint "src-not-empty"; update the
element so its src uses a small placeholder data URI (e.g., a 1x1 transparent
GIF or SVG data URI) instead of an empty string, preserving the class and alt
attributes so the preview loads validly before a real image is set.
In `@frontend/src/components/ui/dialog.tsx`:
- Around line 51-55: The X icon in the close button has a hard-coded
"text-black" class which overrides parent/theme state colors and breaks dark
mode; locate the X element in frontend/src/components/ui/dialog.tsx (the JSX
with <X className="... text-black" />) and remove or replace "text-black" with a
neutral class that inherits the button color (e.g., use "text-current" or omit
color classes) so the parent's state/themed classes (data-[state=open]:...,
focus:ring-..., etc.) control the icon color instead.
In `@frontend/src/pages/gallery/components/GalleryImage.tsx`:
- Line 1: The import line currently lists React hooks in the wrong order; update
the named import from "import { useState, useEffect } from 'react'" to list
hooks in the expected order (e.g., "useEffect" before "useState") so
Prettier/linters accept the file (change the import statement in
GalleryImage.tsx to import { useEffect, useState } from 'react').
In `@frontend/src/pages/gallery/components/GalleryMasonry.tsx`:
- Around line 28-35: The clickable card in GalleryMasonry uses a plain <div>
with onClick (inside the GalleryMasonry component, key={index} block) which is
not keyboard-accessible; replace the interactive <div> with a semantic
interactive element (e.g., a <button> or <a> depending on semantics) or make it
keyboard-focusable and operable by adding role="button", tabIndex={0}, and key
handlers that invoke setSelectedPhoto(photo), and ensure appropriate ARIA
attributes (aria-label or aria-haspopup) are added so the modal can be opened by
keyboard and screen readers.
---
Nitpick comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt`:
- Line 127: The redirect currently appends uploadedPhotos into the query string
(return
"redirect:/admin/control/gallery-upload?uploaded=${uploadedPhotos.joinToString(",").urlEncode()}"),
which can overflow URLs and leak data; modify the controller method in
GalleryUploadController to accept a RedirectAttributes parameter
(org.springframework.web.servlet.mvc.support.RedirectAttributes) and call
redirectAttributes.addFlashAttribute("uploadedPhotos", uploadedPhotos) (or a
concise DTO/list name) instead of encoding into the URL, then change the return
to "redirect:/admin/control/gallery-upload" and remove the
urlEncode/joinToString usage so the upload feedback is carried via flash
attributes rather than the query string.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 390ae906-f59d-477b-b898-0ebbf8907869
📒 Files selected for processing (19)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/task/TaskImageRescaleService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/FileUploadController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/util/Utility.ktbackend/src/main/resources/templates/uploadGallery.htmlfrontend/src/App.tsxfrontend/src/api/contexts/config/types.tsfrontend/src/api/hooks/gallery/useGalleryQuery.tsfrontend/src/api/hooks/queryKeys.tsfrontend/src/assets/kir-dev-logo.tsxfrontend/src/components/ui/dialog.tsxfrontend/src/pages/gallery/components/GalleryImage.tsxfrontend/src/pages/gallery/components/GalleryMasonry.tsxfrontend/src/pages/gallery/gallery.page.tsxfrontend/src/util/language.tsfrontend/src/util/paths.tsfrontend/src/util/views/gallery.view.ts
💤 Files with no reviewable changes (1)
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/task/TaskImageRescaleService.kt
Comment on lines
+88
to
+98
| if (names.size != files.size) { | ||
| throw IllegalArgumentException("The length of names and files does not match") | ||
| } | ||
|
|
||
| val uploadedPhotos = mutableListOf<String>() | ||
| for (i in files.indices) { | ||
| val file = files[i] | ||
| val name = names[i] | ||
| val description = descriptions.getOrNull(i) ?: "" | ||
| val isHighlighted = highlighted.getOrNull(i) ?: false | ||
| val isShowOnHomePage = showOnHomePage.getOrNull(i) ?: false |
There was a problem hiding this comment.
Validate all per-image request lists, not just names.
Only names is length-checked against files. If descriptions, highlighted, or showOnHomePage is shorter/misaligned, metadata can be attached to the wrong photo silently.
Suggested fix
- if (names.size != files.size) {
- throw IllegalArgumentException("The length of names and files does not match")
- }
+ if (names.size != files.size ||
+ descriptions.size != files.size ||
+ highlighted.size != files.size ||
+ showOnHomePage.size != files.size
+ ) {
+ throw IllegalArgumentException("The length of metadata lists and files does not match")
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt`
around lines 88 - 98, The upload logic in GalleryUploadController currently only
checks names.size against files.size, allowing descriptions, highlighted, or
showOnHomePage to be shorter/misaligned and silently produce wrong metadata;
update the validation to require all per-image lists (names, descriptions,
highlighted, showOnHomePage) either be equal to files.size or be entirely
absent/empty, and if not throw an IllegalArgumentException. Locate the method
using variables names, files, descriptions, highlighted, showOnHomePage (the
upload loop building uploadedPhotos) and add size checks for descriptions.size,
highlighted.size, and showOnHomePage.size (or normalize inputs up-front by
expanding/trimming to files.size) so metadata indices always align with files.
Comment on lines
+108
to
+124
| storageService.saveNamedObject("gallery", fileName, file).ifPresent { url -> | ||
| val thumbUrl = generateThumbnail(file, extension)?.let { thumbData -> | ||
| storageService.saveNamedObject("gallery", thumbName, file.contentType ?: "image/jpeg", thumbData) | ||
| .orElse("") | ||
| } ?: "" | ||
|
|
||
| val entity = GalleryEntity( | ||
| title = name, | ||
| description = description, | ||
| highlighted = isHighlighted, | ||
| showOnHomePage = isShowOnHomePage, | ||
| url = url, | ||
| thumbnailUrl = thumbUrl | ||
| ) | ||
| galleryService.savePhoto(entity) | ||
| uploadedPhotos.add(name) | ||
| } |
There was a problem hiding this comment.
Do not persist gallery entries when thumbnail generation fails.
The current flow can still save the original file and DB row with an empty thumbnailUrl after thumbnail failure. This creates broken gallery cards and weakens upload validation.
Suggested fix
- storageService.saveNamedObject("gallery", fileName, file).ifPresent { url ->
- val thumbUrl = generateThumbnail(file, extension)?.let { thumbData ->
- storageService.saveNamedObject("gallery", thumbName, file.contentType ?: "image/jpeg", thumbData)
- .orElse("")
- } ?: ""
+ val thumbData = generateThumbnail(file, extension)
+ if (thumbData == null || thumbData.isEmpty()) {
+ log.warn("Skipping upload for '{}': thumbnail generation failed", name)
+ continue
+ }
+
+ storageService.saveNamedObject("gallery", fileName, file).ifPresent { url ->
+ val thumbUrl = storageService
+ .saveNamedObject("gallery", thumbName, file.contentType ?: "image/jpeg", thumbData)
+ .orElse("")
+ if (thumbUrl.isBlank()) {
+ log.warn("Skipping DB save for '{}': thumbnail upload failed", name)
+ return@ifPresent
+ }
val entity = GalleryEntity(Also applies to: 130-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt`
around lines 108 - 124, The code currently saves the original file and DB row
even if thumbnail generation or thumbnail storage fails; change the flow so you
generate the thumbnail first (call generateThumbnail(file, extension)) and
attempt to save the thumbnail with storageService.saveNamedObject("gallery",
thumbName, ...), and only if that returns a non-empty URL proceed to save the
original (storageService.saveNamedObject("gallery", fileName, file)) and then
create the GalleryEntity and call galleryService.savePhoto(entity); ensure you
do not create the GalleryEntity or call savePhoto when generateThumbnail returns
null or the thumbnail save returns empty (and if you must keep current order,
ensure you delete the original via the storage service when thumbnail save
fails).
Comment on lines
+108
to
+123
| storageService.saveNamedObject("gallery", fileName, file).ifPresent { url -> | ||
| val thumbUrl = generateThumbnail(file, extension)?.let { thumbData -> | ||
| storageService.saveNamedObject("gallery", thumbName, file.contentType ?: "image/jpeg", thumbData) | ||
| .orElse("") | ||
| } ?: "" | ||
|
|
||
| val entity = GalleryEntity( | ||
| title = name, | ||
| description = description, | ||
| highlighted = isHighlighted, | ||
| showOnHomePage = isShowOnHomePage, | ||
| url = url, | ||
| thumbnailUrl = thumbUrl | ||
| ) | ||
| galleryService.savePhoto(entity) | ||
| uploadedPhotos.add(name) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== StorageService definition =="
fd -i 'StorageService.kt' | xargs -I{} sh -c 'echo "--- {} ---"; sed -n "1,240p" "{}"'
echo
echo "== saveNamedObject call sites =="
rg -n -C3 '\bsaveNamedObject\s*\(' --type=kt
echo
echo "== Potential storage cleanup APIs =="
rg -n -C2 '\b(delete|remove|cleanup|purge)\w*\s*\(' --type=ktRepository: kir-dev/cmsch
Length of output: 14196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Other saveNamedObject usage in controllers =="
rg -n 'saveNamedObject' --type kotlin | head -30
echo
echo "== GalleryService.savePhoto implementation =="
fd -i 'GalleryService.kt' | xargs -I{} sh -c 'cat -n "{}"'
echo
echo "== GalleryUploadController full context (lines 100-130) =="
sed -n '100,130p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt
echo
echo "== Search for try-catch around savePhoto calls =="
rg -n 'savePhoto' --type kotlin -B3 -A3 | head -50Repository: kir-dev/cmsch
Length of output: 7510
Add compensation for storage/DB partial failures.
If object storage succeeds but galleryService.savePhoto(entity) fails, orphaned files remain in storage. The @Transactional annotation on savePhoto() only protects the database operation, not the prior storage writes. This endpoint needs error handling to clean up uploaded files if the database save fails.
StorageService provides deleteObject() and delete() methods to support cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt`
around lines 108 - 123, Wrap the call to galleryService.savePhoto(entity) in a
try/catch and if it throws, remove any already-stored objects by calling
storageService.deleteObject("gallery", fileName) and, if a thumbnail was stored
(thumbUrl non-empty), storageService.deleteObject("gallery", thumbName) (or
storageService.delete equivalents) before rethrowing or converting to an
appropriate error response; ensure you reference the existing locals (fileName,
thumbName, url, thumbUrl), perform cleanup only when storage succeeded, and
avoid swallowing the original exception so callers still get the failure.
Comment on lines
+126
to
+132
| val scalingFactor = minOf(maxWidth.toDouble() / width, maxHeight.toDouble() / height) | ||
|
|
||
| val newWidth = (width * scalingFactor).toInt() | ||
| val newHeight = (height * scalingFactor).toInt() | ||
|
|
||
| val resizedImage = BufferedImage(newWidth, newHeight, BufferedImage.TYPE_INT_RGB) | ||
|
|
There was a problem hiding this comment.
Prevent zero-sized output dimensions during resize.
On Line 128 and Line 129, flooring can produce 0 for extreme aspect ratios (e.g., very thin/tall images), which will fail at Line 131 when constructing BufferedImage.
Suggested fix
- val newWidth = (width * scalingFactor).toInt()
- val newHeight = (height * scalingFactor).toInt()
+ val newWidth = (width * scalingFactor).toInt().coerceAtLeast(1)
+ val newHeight = (height * scalingFactor).toInt().coerceAtLeast(1)📝 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
| val scalingFactor = minOf(maxWidth.toDouble() / width, maxHeight.toDouble() / height) | |
| val newWidth = (width * scalingFactor).toInt() | |
| val newHeight = (height * scalingFactor).toInt() | |
| val resizedImage = BufferedImage(newWidth, newHeight, BufferedImage.TYPE_INT_RGB) | |
| val scalingFactor = minOf(maxWidth.toDouble() / width, maxHeight.toDouble() / height) | |
| val newWidth = (width * scalingFactor).toInt().coerceAtLeast(1) | |
| val newHeight = (height * scalingFactor).toInt().coerceAtLeast(1) | |
| val resizedImage = BufferedImage(newWidth, newHeight, BufferedImage.TYPE_INT_RGB) | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/main/kotlin/hu/bme/sch/cmsch/util/Utility.kt` around lines 126 -
132, The computed newWidth/newHeight can become 0 for extreme aspect ratios
before creating the BufferedImage; clamp the values after calculating
scalingFactor by ensuring newWidth and newHeight are at least 1 (e.g., replace
the direct toInt results with max(1, (width * scalingFactor).toInt()) and max(1,
(height * scalingFactor).toInt())) so the BufferedImage(...) call never receives
zero dimensions; update the calculations around scalingFactor, newWidth,
newHeight to enforce this minimum.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/pages/gallery/components/GalleryImage.tsx (1)
30-43: Consider adding keyboard support ifonClickis used for interactive purposes.SonarCloud flags this as an accessibility concern. Currently,
GalleryMasonrydoesn't passonClickto this component (the parentdivhandles clicks), so this is not a blocking issue. However, ifonClickis intended for direct image interaction in the future, consider making the<img>focusable and adding keyboard handlers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/gallery/components/GalleryImage.tsx` around lines 30 - 43, The <GalleryImage> component's <img> lacks keyboard accessibility when an interactive onClick prop is used; update the img (referenced by handleImageRef) to be focusable and respond to keyboard activation only when onClick is provided: add tabIndex={0} and role="button" conditionally, and implement an onKeyDown handler that calls onClick when Enter or Space is pressed; ensure existing state/props (loaded, setLoaded, src, alt, className, aspectRatio) remain unchanged and only apply these attributes/handlers when onClick is truthy.frontend/src/pages/gallery/components/GalleryMasonry.tsx (1)
29-29: Prefer stable keys over array indices.Using array index as a React key can cause issues with component state if the list order changes. Since
photo.urlshould be unique, consider using it as the key instead.Suggested fix
- key={index} + key={photo.url}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/gallery/components/GalleryMasonry.tsx` at line 29, The component GalleryMasonry uses array index as the React key (key={index}), which is unstable; change the key to a stable unique identifier such as photo.url (e.g., key={photo.url}) wherever the map/render of photos occurs so each <GalleryMasonry> child uses a stable key; ensure photo.url is present/unique or fall back to another unique id field before replacing the index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/pages/gallery/components/GalleryImage.tsx`:
- Around line 30-43: The <GalleryImage> component's <img> lacks keyboard
accessibility when an interactive onClick prop is used; update the img
(referenced by handleImageRef) to be focusable and respond to keyboard
activation only when onClick is provided: add tabIndex={0} and role="button"
conditionally, and implement an onKeyDown handler that calls onClick when Enter
or Space is pressed; ensure existing state/props (loaded, setLoaded, src, alt,
className, aspectRatio) remain unchanged and only apply these
attributes/handlers when onClick is truthy.
In `@frontend/src/pages/gallery/components/GalleryMasonry.tsx`:
- Line 29: The component GalleryMasonry uses array index as the React key
(key={index}), which is unstable; change the key to a stable unique identifier
such as photo.url (e.g., key={photo.url}) wherever the map/render of photos
occurs so each <GalleryMasonry> child uses a stable key; ensure photo.url is
present/unique or fall back to another unique id field before replacing the
index.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 90c051c2-8ba6-4a2e-9f5d-518f12519b5f
📒 Files selected for processing (19)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/task/TaskImageRescaleService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/FileUploadController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/util/Utility.ktbackend/src/main/resources/templates/uploadGallery.htmlfrontend/src/App.tsxfrontend/src/api/contexts/config/types.tsfrontend/src/api/hooks/gallery/useGalleryQuery.tsfrontend/src/api/hooks/queryKeys.tsfrontend/src/assets/kir-dev-logo.tsxfrontend/src/components/ui/dialog.tsxfrontend/src/pages/gallery/components/GalleryImage.tsxfrontend/src/pages/gallery/components/GalleryMasonry.tsxfrontend/src/pages/gallery/gallery.page.tsxfrontend/src/util/language.tsfrontend/src/util/paths.tsfrontend/src/util/views/gallery.view.ts
💤 Files with no reviewable changes (1)
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/task/TaskImageRescaleService.kt
✅ Files skipped from review due to trivial changes (8)
- frontend/src/assets/kir-dev-logo.tsx
- frontend/src/util/views/gallery.view.ts
- frontend/src/api/hooks/queryKeys.ts
- frontend/src/api/contexts/config/types.ts
- backend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/FileUploadController.kt
- frontend/src/components/ui/dialog.tsx
- frontend/src/util/language.ts
- frontend/src/pages/gallery/gallery.page.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/App.tsx
- frontend/src/api/hooks/gallery/useGalleryQuery.ts
- frontend/src/util/paths.ts
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponent.kt
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryEntity.kt
Isti01
force-pushed
the
feature/gallery-frontend
branch
from
e0d5806 to
c128d77
Compare
Isti01
force-pushed
the
feature/gallery-frontend
branch
from
c128d77 to
d4d7375
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
backend/src/main/resources/templates/uploadGallery.html (2)
53-54: Consider using standard Thymeleaf link expression syntax.The form action mixes literal substitution with a partial link expression. While functional, the idiomatic approach is cleaner:
- <form id="file-form" th:action="|@{/admin/control}/gallery-upload|" method="post" + <form id="file-form" th:action="@{/admin/control/gallery-upload}" method="post"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/resources/templates/uploadGallery.html` around lines 53 - 54, The form's th:action uses a mixed literal substitution pattern; update the form element with id "file-form" to use the standard Thymeleaf link expression syntax for the action (e.g., change the current th:action value to a single @{} expression pointing to the "/admin/control/gallery-upload" endpoint) so the form action is expressed idiomatically and consistently.
155-163: Addidattributes to inputs andforattributes to labels for accessibility.The "Cím" and "Leírás" labels are not programmatically associated with their inputs, which impacts screen reader accessibility. Since these are in a cloned template, you can add base IDs and update them in JavaScript similar to the checkbox handling.
<div class="field-group"> - <label>Cím</label> - <input type="text" class="name-input" name="names" autocomplete="off" /> + <label class="name-label">Cím</label> + <input type="text" class="name-input" name="names" autocomplete="off" /> </div> <div class="field-group"> - <label>Leírás</label> - <textarea name="descriptions" style="width: 100%; min-height: 50px;"></textarea> + <label class="desc-label">Leírás</label> + <textarea class="desc-input" name="descriptions" style="width: 100%; min-height: 50px;"></textarea> </div>Then in the JavaScript, add associations like the checkbox handling:
const nameInput = fileNameInput.querySelector('.name-input'); const nameLabel = fileNameInput.querySelector('.name-label'); const nameId = `name_${i}`; nameInput.id = nameId; nameLabel.setAttribute('for', nameId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/resources/templates/uploadGallery.html` around lines 155 - 163, The labels for the "Cím" and "Leírás" fields are not associated with their inputs; add id attributes to the input (class name-input / name="names") and textarea (name="descriptions") and add matching for attributes to their labels (e.g., give labels classes like name-label and description-label) so they can be linked programmatically; in the cloning JS (same place you handle the checkbox) set unique ids per clone (e.g., name_${i}, description_${i}) and assign nameInput.id/nameLabel.setAttribute('for', id) and descriptionTextarea.id/descriptionLabel.setAttribute('for', id) to mirror the checkbox handling.frontend/src/pages/gallery/components/GalleryImage.tsx (1)
33-40: Add accessibility support for clickable images.When
onClickis provided, the image becomes interactive but lacks keyboard accessibility. Users navigating with keyboards or assistive technologies cannot activate the click handler.Consider conditionally adding
tabIndex,role,cursorstyling, and keyboard support whenonClickis present.♿ Proposed accessibility improvement
<img ref={handleImageRef} src={src} alt={alt} - className={cn('h-auto w-full object-cover', loaded ? 'block' : 'hidden')} + className={cn('h-auto w-full object-cover', loaded ? 'block' : 'hidden', onClick && 'cursor-pointer')} onLoad={() => setLoaded(true)} onClick={onClick} + onKeyDown={onClick ? (e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); onClick(); } } : undefined} + tabIndex={onClick ? 0 : undefined} + role={onClick ? 'button' : undefined} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/gallery/components/GalleryImage.tsx` around lines 33 - 40, The image is interactive when onClick is provided but lacks keyboard/ARIA affordances; update the GalleryImage component to, when onClick is present, add tabIndex={0}, role="button", a cursor-pointer style/class (e.g., via cn), and an onKeyDown handler that triggers the same handler used by onClick for Enter and Space keys; ensure you reference the img element props (ref={handleImageRef}, onLoad={() => setLoaded(true)}, src, alt) and wire the onClick function so keyboard activation calls it consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/src/main/resources/templates/uploadGallery.html`:
- Around line 53-54: The form's th:action uses a mixed literal substitution
pattern; update the form element with id "file-form" to use the standard
Thymeleaf link expression syntax for the action (e.g., change the current
th:action value to a single @{} expression pointing to the
"/admin/control/gallery-upload" endpoint) so the form action is expressed
idiomatically and consistently.
- Around line 155-163: The labels for the "Cím" and "Leírás" fields are not
associated with their inputs; add id attributes to the input (class name-input /
name="names") and textarea (name="descriptions") and add matching for attributes
to their labels (e.g., give labels classes like name-label and
description-label) so they can be linked programmatically; in the cloning JS
(same place you handle the checkbox) set unique ids per clone (e.g., name_${i},
description_${i}) and assign nameInput.id/nameLabel.setAttribute('for', id) and
descriptionTextarea.id/descriptionLabel.setAttribute('for', id) to mirror the
checkbox handling.
In `@frontend/src/pages/gallery/components/GalleryImage.tsx`:
- Around line 33-40: The image is interactive when onClick is provided but lacks
keyboard/ARIA affordances; update the GalleryImage component to, when onClick is
present, add tabIndex={0}, role="button", a cursor-pointer style/class (e.g.,
via cn), and an onKeyDown handler that triggers the same handler used by onClick
for Enter and Space keys; ensure you reference the img element props
(ref={handleImageRef}, onLoad={() => setLoaded(true)}, src, alt) and wire the
onClick function so keyboard activation calls it consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17f08316-e636-47dc-8da9-baf97d2de370
📒 Files selected for processing (20)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponentController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/task/TaskImageRescaleService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/FileUploadController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/util/Utility.ktbackend/src/main/resources/templates/uploadGallery.htmlfrontend/src/App.tsxfrontend/src/api/contexts/config/types.tsfrontend/src/api/hooks/gallery/useGalleryQuery.tsfrontend/src/api/hooks/queryKeys.tsfrontend/src/assets/kir-dev-logo.tsxfrontend/src/components/ui/dialog.tsxfrontend/src/pages/gallery/components/GalleryImage.tsxfrontend/src/pages/gallery/components/GalleryMasonry.tsxfrontend/src/pages/gallery/gallery.page.tsxfrontend/src/util/language.tsfrontend/src/util/paths.tsfrontend/src/util/views/gallery.view.ts
💤 Files with no reviewable changes (1)
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/task/TaskImageRescaleService.kt
✅ Files skipped from review due to trivial changes (9)
- frontend/src/App.tsx
- frontend/src/util/language.ts
- frontend/src/util/views/gallery.view.ts
- frontend/src/assets/kir-dev-logo.tsx
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponentController.kt
- backend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/FileUploadController.kt
- frontend/src/pages/gallery/gallery.page.tsx
- frontend/src/api/hooks/queryKeys.ts
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryUploadController.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/src/components/ui/dialog.tsx
- frontend/src/api/hooks/gallery/useGalleryQuery.ts
- frontend/src/util/paths.ts
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryComponent.kt
- backend/src/main/kotlin/hu/bme/sch/cmsch/util/Utility.kt
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/gallery/GalleryEntity.kt
- frontend/src/pages/gallery/components/GalleryMasonry.tsx
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.
This is how it looks
Summary by CodeRabbit