Repository navigation
Conversation
…l uploads The media pages get their own tiles layout: the folder list replaces the Powered-by card in the sidebar, and a viewport meta tag lets the page reflow on phones, where a folder picker replaces the list. Files show as equal cards in a CSS grid. Long names wrap in two lines, and each card shows type, size and a selection checkbox with an accessible label. Search, the selection toolbar (select page, clear, move, delete) and the Upload button now sit above the files. Uploading several files no longer hides the ones that were saved when another fails: the success page lists the failures with their names and reasons. The unused Name field and the broken Cancel, Upload more and No enclosure controls are fixed.
Browsing a folder and searching now run the same paged query (48 files a page) with an explicit order and the ID as tie-breaker, so pages never repeat or skip files. Newest first is the default. Search can cover the current folder or all folders, matches names that start with the text in any case, and is a GET, so the page state lives in the URL. The shared pager says Previous and Next for media. The library offers a list view (name, type, size, uploaded) beside the grid, remembered in the browser. Actions on the selection return to the same folder, search, sort and page. File details move from an iframe dialog to their own page: preview, facts, Copy URL, Open original, Create entry and Delete, then the metadata form, with a link back to the library page the user came from.
…failed files The upload page has one multiple-file picker and a drop area. Chosen files go into a list with a preview, name, size and a remove button; the shared description, tags and copyright come after the files, and each file keeps its own name. The page shows the configured limits. With JavaScript the files go in one request and MediaFileAddResult.jsp answers with a result per file and a fresh salt, so a retry sends only the files that failed and never uploads a saved file twice. If the connection drops or the server fails after the files were sent, the outcome is reported as unknown, with a link to the folder, and nothing is retried. Without JavaScript the form posts as before. The server caps one upload at uploads.maxFilesPerRequest files (20).
The entry editor's media chooser uses the library's card and query: a page of a folder, newest first, with a name search and the shared pager. Inserting an image uses its description as the alt text (the file name when there is none), and the editor builds the inserted markup with DOM APIs, so names and descriptions are escaped. Create entry escapes them too. The description field is labeled as the alt text. The upload success page shows readable sizes and no longer shows pixel dimensions for non-images. Old media styles are removed, and the user guide describes the new library, upload, details and chooser.
The card hides on narrow screens, where the folder picker takes the sidebar's place, so the files stay near the top of the page.
WeblogMediaLibraryIT covers the library's main uses with the journey's account and weblog: uploading several files at once, keeping saved files when another fails and retrying only the failed one, moving files between folders, searching by name in a folder or all folders, paging a folder of 50 files without repeats or gaps, the remembered list view, editing a file on its details page and returning to the same library page, deleting files, and inserting a file into an entry with its description as the alt text. The new-user journey picks its upload from the new file picker.
Sort order and layout move up beside the file count, so the selection count no longer wraps the toolbar and pushes the files down. The upload form's spacing rule targeted a fieldset that the simple theme does not render.
|
This is a huge PR that needs careful review |
The upload results page (mediaFileAdd!done) only displays data, so it is listed with the other methods a GET may call. MediaFileEditTest gets a URL strategy, since save() now shows the saved file again.
snoopdave
left a comment
snoopdave
left a comment
There was a problem hiding this comment.
🐞Codex Issue: PR-Review: 4 inline findings (1 Blocking, 3 Important), each reproduced against this PR in a local Chromium/Jetty run. Validation: app verification and WAR packaging passed (595 tests, 0 failures/errors, 1 skipped); NewUserJourneyIT and all 9 WeblogMediaLibraryIT tests passed. The findings cover cases outside that suite. One general changelog finding is posted separately.
|
🐞Codex Issue: PR-Review: General Issues
|
snoopdave
left a comment
snoopdave
left a comment
There was a problem hiding this comment.
PR-Review: 3 inline comments posted.
|
PR-Review: General Issues The following issues were found but cannot be attached to a specific line in the diff:
|
|
A 37-second tour of the redesigned media library, recorded from this branch: creating a folder, uploading several files at once, grid and list views, type filters, search, sort, editing a file's details, and selecting files to move or delete. roller-media-library-demo.webm |
- Refuse new files while an upload runs; leave the page only when every file uploaded - Keep the details form after a validation error; keep the folder after Upload more - Keep the new-folder form on narrow screens; cap the page number - Build the library link from this weblog; remove a duplicate Javadoc - Restore CRLF line endings in three files; add a CHANGES.md entry
|
🤖Claude: in dd2cf48:
|
mraible
left a comment
mraible
left a comment
There was a problem hiding this comment.
Nice redesign. Keeping library state in the URL, the JSON upload path with a non-JS fallback, and the case-insensitive name search all look good, and CI is green. LGTM, with three small non-blocking notes inline.
| @StrutsParameter | ||
| public void setReturnQuery(String returnQuery) { | ||
| this.returnQuery = returnQuery != null && returnQuery.length() <= 2000 | ||
| && returnQuery.matches("[A-Za-z0-9._~%&=+-]*") ? returnQuery : null; |
There was a problem hiding this comment.
Non-blocking: URLEncoder leaves * unencoded, so a library search for IMG* produces a returnQuery of bean.name=IMG*. This regex rejects it, and Back to library drops the search, sort and page. Adding * to the class ([A-Za-z0-9._~%&=+*-]*) covers everything getLibraryQuery() can produce.
| } | ||
| // Files that were saved are listed on the success page, with | ||
| // any that failed, so a partial upload is never hidden. | ||
| this.pageTitle = "mediaFileAddSuccess.title"; |
There was a problem hiding this comment.
Non-blocking: when a non-JS upload partly succeeds, each failed file shows twice. It appears once in the error box from messages.jsp (via failed()'s addActionError) and again in the success page's failed list. To reproduce, upload a.png plus a disallowed b.exe with JavaScript off. Calling clearActionErrors() before returning SUCCESS here, or only adding the action error when the result is INPUT, would list it once.
| do { | ||
| value /= 1024; | ||
| unit++; | ||
| } while (value >= 1024 && unit < UNITS.length - 1); |
There was a problem hiding this comment.
Non-blocking: the unit is picked before rounding, so 1048575 bytes shows as "1024 KB" instead of "1 MB" (and 1073741823 as "1024 MB"). Continuing the loop while value >= 1023.5, the point where %.0f rounds up to 1024, would fix it. A test case for 1048575 bytes would catch it.
A redesign of the media library and file upload pages for Roller 7. The changes in this PR are mostly about:
Tested locally: the unit tests and the Playwright UI suite pass, including a new
WeblogMediaLibraryITwith 9 tests for the media pages. The user guide is updated; its old media screenshots are no longer referenced and need replacing.(It is large because it replaces the media pages, their CSS and scripts, and adds the tests.)