diff --git a/api/handlers/image/imageHandler.go b/api/handlers/image/imageHandler.go deleted file mode 100644 index 9ac5f1e..0000000 --- a/api/handlers/image/imageHandler.go +++ /dev/null @@ -1,74 +0,0 @@ -package image - -import ( - "RenewCMS/api/pkg/apperr" - "RenewCMS/api/pkg/claims" - "RenewCMS/api/pkg/upload" - domainArticle "RenewCMS/internal/domain/article" - "RenewCMS/internal/infrastructure/useCases" - "encoding/json" - "errors" - "net/http" - "strconv" - - "github.com/go-chi/chi/v5" -) - -type Handler struct { - CreateUseCase *useCases.CreateImageUseCase - GetUseCase *useCases.GetArticleUseCase -} - -func NewHandler(create *useCases.CreateImageUseCase, get *useCases.GetArticleUseCase) *Handler { - return &Handler{ - CreateUseCase: create, - GetUseCase: get, - } -} - -func writeInternalServerError(w http.ResponseWriter) { - http.Error(w, "Internal server error", http.StatusInternalServerError) -} - -func writeArticleError(w http.ResponseWriter, err error) { - if errors.Is(err, domainArticle.ErrArticleNotFound) { - http.Error(w, apperr.ArticleNotFoundErrorMessage, http.StatusNotFound) - return - } - writeInternalServerError(w) -} - -func (h *Handler) PostLegacyArticleImage(w http.ResponseWriter, r *http.Request) { - articleId, err := strconv.ParseUint(chi.URLParam(r, "id"), 10, 32) - if err != nil { - http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) - return - } - - if _, err := h.GetUseCase.GetArticle(uint32(articleId)); err != nil { - writeArticleError(w, err) - return - } - - userId, _ := claims.UserId(r) - input, closeFile, ok := upload.ReadImageInput(w, r, userId) - if !ok { - return - } - defer closeFile() - - newImage, err := h.CreateUseCase.CreateImage(input) - if err != nil { - upload.WriteCreateError(w, err) - return - } - - responseJson, err := json.Marshal(map[string]any{"id": newImage.ID, "location": newImage.Path}) - if err != nil { - writeInternalServerError(w) - return - } - - w.WriteHeader(http.StatusCreated) - _, _ = w.Write(responseJson) -} diff --git a/api/router.go b/api/router.go index 03e4c7f..0352af8 100644 --- a/api/router.go +++ b/api/router.go @@ -4,7 +4,6 @@ import ( "RenewCMS/api/handlers/article" "RenewCMS/api/handlers/auth" "RenewCMS/api/handlers/frontend" - "RenewCMS/api/handlers/image" "RenewCMS/api/handlers/media" "RenewCMS/api/handlers/settings" "RenewCMS/api/handlers/staticimages" @@ -36,7 +35,6 @@ func NewArticleRouter(container *UseCases) http.Handler { r := chi.NewRouter() articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase, container.UpdateArticleUseCase, container.GetUserUseCase) - imageHandler := image.NewHandler(container.CreateImageUseCase, container.GetArticleUseCase) r.Group(func(r chi.Router) { r.Get("/", articleHandler.ListArticles) @@ -51,7 +49,6 @@ func NewArticleRouter(container *UseCases) http.Handler { r.Patch("/{id}", articleHandler.UpdateArticleBody) r.Patch("/{id}/publish", articleHandler.PublishArticle) r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle) - r.Post("/{id}/image", imageHandler.PostLegacyArticleImage) }) return r } diff --git a/docs/architecture.md b/docs/architecture.md index 8a69ef9..b699de3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -27,7 +27,6 @@ Nothing under `internal/` imports `api/`. |---|---| | `api/handlers/user` | Authenticated user endpoints: profile update (`UpdateMe`) and the users list with invitations (`CreateInvite`, `ListUsers`). | | `api/handlers/media` | The media library under `/v1/media` (list, upload, rename, delete, merge, usage). Images belong to the library, not to an article: deleting an article never deletes an image, and an image's "used in" is read from article bodies each time. | -| `api/handlers/image` | `POST /v1/article/{id}/image`, a temporary alias that creates a library image and attaches nothing. `PostLegacyArticleImage` goes away when the editor moves to `/v1/media`. | | `api/pkg/upload` | Multipart `file` reading and the mapping of upload errors (415, 413, 400) shared by both upload routes. | | `api/pkg/publicuser` | `PublicUser`, the JSON shape of a user with no password, codes or token version. `publicuser.New` converts a domain `user.User`, so handlers never serialise the domain struct directly. | diff --git a/features/editor_image_crop_resize.feature b/features/editor_image_crop_resize.feature index 76cbeb8..9fa6441 100644 --- a/features/editor_image_crop_resize.feature +++ b/features/editor_image_crop_resize.feature @@ -9,8 +9,10 @@ Feature: Crop and resize step in the editor's image uploader Given ImageUploader inserts a crop-and-resize step between the idle file picker and the upload, for any JPEG, PNG or WEBP that already passed the browser validation described in features/editor_image_upload.feature; that feature's idle, dragging-over, rejected, - uploading, inserted and failed states and its POST /v1/article/{articleId}/image contract - are unchanged and out of scope here + uploading, inserted and failed states and its POST /v1/media/ contract are unchanged and + out of scope here + And the hash lookup from features/editor_image_upload.feature runs before this step opens, + so the step only opens for a file with no match, or after "Upload anyway" And an SVG file skips this step entirely and uploads immediately, as before And opening the step retitles the modal from "Add an image" to "Crop and resize", with the description "{filename} · {size}" (e.g. "north-wall.jpg · 3.9 MB") @@ -32,8 +34,8 @@ Feature: Crop and resize step in the editor's image uploader crop is narrower than the chosen width, an info row reads "The crop is {crop width} px wide, so it's uploaded at that size. Enlarging it would make it blurry." And the footer offers "Choose another file" (discards this step and returns to the idle file - picker with no adjustments carried over) and "Upload image" (uploads the cropped, resized - file through the unchanged POST /v1/article/{articleId}/image flow) + picker, with the tabs, with no adjustments carried over) and "Upload image" (uploads the cropped, resized + file through the unchanged POST /v1/media/ flow) And "Rotate" turns the image 90° clockwise each activation, cycling 0°→90°→180°→270°→0°, and width presets apply to the post-rotation image And "Reset" returns the ratio to Original, the width to 1600 px, the crop frame to the whole @@ -43,7 +45,7 @@ Feature: Crop and resize step in the editor's image uploader Given ImageUploader is open in the idle state for article 12 When the user chooses a file named "north-wall.jpg", content type "image/jpeg", size 3.9 MB, natural dimensions 4032 × 3024 px - Then no request is sent to POST /v1/article/12/image + Then no request is sent to POST /v1/media/ And the modal retitles to "Crop and resize" with the description "north-wall.jpg · 3.9 MB" And the crop frame starts as the whole image, ratio "Original" is selected, and width "1600 px" is selected @@ -53,7 +55,7 @@ Feature: Crop and resize step in the editor's image uploader Scenario: An SVG skips the crop step and uploads immediately Given ImageUploader is open in the idle state for article 12 When the user chooses a file named "diagram.svg", content type "image/svg+xml", size 40 KB - Then POST /v1/article/12/image is sent immediately with that file + Then POST /v1/media/ is sent immediately with that file And the crop-and-resize step is never shown Scenario Outline: Choosing an aspect ratio locks the frame to it @@ -108,7 +110,7 @@ Feature: Crop and resize step in the editor's image uploader Scenario: "Choose another file" returns to the file picker without uploading Given the crop-and-resize step is open for "north-wall.jpg" When the user activates "Choose another file" - Then no request is sent to POST /v1/article/12/image + Then no request is sent to POST /v1/media/ And the idle state is shown again, with the file picker available Scenario: A newly chosen file opens a fresh crop step, not the previous file's adjustments @@ -123,7 +125,7 @@ Feature: Crop and resize step in the editor's image uploader Given the crop-and-resize step is open for "north-wall.jpg" with ratio "Original", width "1600 px" and the mono line "Uploads as 1600 × 1200 px · about 420 KB" When the user activates "Upload image" - Then POST /v1/article/12/image is sent with a single file under the field name "file" whose + Then POST /v1/media/ is sent with a single file under the field name "file" whose pixel dimensions are 1600 × 1200 and whose size is about 420 KB, not the original 4032 × 3024, 3.9 MB file And the uploading state shows "Uploading 0.0 of 420 KB" diff --git a/features/editor_image_crop_resize.qa.md b/features/editor_image_crop_resize.qa.md index d4b9f00..9578a0b 100644 --- a/features/editor_image_crop_resize.qa.md +++ b/features/editor_image_crop_resize.qa.md @@ -18,7 +18,7 @@ it renders internally (canvas/OffscreenCanvas is a coder implementation detail). "1600 px" selected, mono line "Uploads as 1600 × 1200 px · about 420 KB", footer "Choose another file" / "Upload image". 2. **An SVG bypasses the step** — choose `diagram.svg` (image/svg+xml, 40 KB); assert an XHR to - `/v1/article/12/image` is created immediately and the crop step never renders. + `/v1/media/` is created immediately and the crop step never renders. 3. **Ratio presets lock the frame, Examples-driven (16:9 / 4:3 / 1:1)** — from the opened step, select each ratio pill; assert the frame's reported rect and the mono line's output size match the feature's Examples table. diff --git a/features/editor_image_removal.feature b/features/editor_image_removal.feature index 00347c4..e5a3ea8 100644 --- a/features/editor_image_removal.feature +++ b/features/editor_image_removal.feature @@ -1,148 +1,90 @@ -Feature: Removing an inline image from the editor +Feature: Removing an inline image from the article As a verified RenewCMS owner writing an article - I want to remove an image I've already inserted into the article body, - either through an explicit delete button or by pressing Backspace/Delete - on the selected image, directly from the editor - So that a mistaken or unwanted image doesn't have to stay in the article - forever, and removing it behaves like every other destructive action in - RenewCMS: it asks for confirmation first, however the user starts it + I want to take an image out of the article body, from a button or with Backspace or Delete + So that an unwanted image leaves the article without deleting the file from my library Background: - Given docs/design/ has no board or state for removing an inline image; - this feature was added at the user's explicit request ahead of design - coverage, reusing the editor's existing visual language rather than - inventing new components: the trash icon and error color already used - by the article "Delete" button, and the ConfirmDialog component - already used by "Delete this article?" (docs/design/screens/editor.md's - "Delete: confirm" state) - And this slice changes ArticleEditor's exposed insertImage(src, alt) to - insertImage(src, alt, imageId), stamping the inserted node with - the image's numeric id as a data-image-id attribute, persisted in the - saved article body HTML; EditorView passes the id it now receives from - a successful upload (features/remove_article_image.feature's change to - POST .../image's 201 body) through to insertImage - And an image already present in an article's body from before this slice - has no data-image-id attribute and so shows no delete affordance and - is not intercepted by Backspace/Delete, until it is removed and - re-added - And selecting an image node that does carry a data-image-id attribute - shows a small delete button anchored to the image's top-right corner, - icon i-lucide-trash-2, color error, matching the article Delete - button's icon and color; deselecting the image (clicking elsewhere) + Given this slice is SPA only for this file; the Go alias deletion is in + features/remove_legacy_article_image_alias.feature + And docs/design/screens/media-picker.md now covers this flow ("Remove image: confirm" and + "Removed"); it supersedes the earlier version of this feature, which had no design + coverage and used a red confirm that deleted through DELETE /v1/article/{id}/image/{imageId} + And removing an image is a body edit only: no API call of any kind is sent, before, during + or after the confirm, and the library and every other article are untouched + And ArticleEditor keeps its exposed insertImage(src, alt, imageId) and removeImage(imageId), + its "request-remove-image" event with the image id, and its Backspace and Delete handling, + which still intercepts the keys before Tiptap deletes the node + And an image already in a body from before data-image-id existed has no id, shows no + "Remove image" button and is not intercepted by Backspace or Delete (unchanged) + And selecting an image node that carries data-image-id shows a button "Remove image" at the + image's top-right corner (8 px from the top and right): trash-2 icon at 14 px, neutral, + 28 px high, 12 px medium text, a 1 px accented border and the page background; deselecting hides it - And there are two equivalent triggers for the same removal flow: - activating that delete button, and pressing Backspace or Delete while - the image node is selected (a Tiptap NodeSelection on the image). - Tiptap's native node deletion on Backspace/Delete is intercepted - before it reaches the document (e.g. via a keyboard-shortcut/keydown - handler registered on the Image node or at the editor level, checked - first while an image node is selected) so the key press never removes - the node directly; instead it triggers exactly the same flow as the - button - And either trigger emits ArticleEditor's new "request-remove-image" - event with the node's numeric image id; ArticleEditor does not call - the API or show any dialog itself, mirroring how "request-image" - already only opens ImageUploader from EditorView - And EditorView reacts to "request-remove-image" by opening the existing - ConfirmDialog component with title "Remove this image?", description - "This image will be removed from the article. This can't be undone.", - confirmLabel "Remove image", cancelLabel "Cancel", pendingLabel - "Removing…", tone "danger" — the same component and tone already used - for "Delete this article?" - And ConfirmDialog's action calls DELETE /v1/article/{articleId}/image/ - {imageId} through useApi (not the raw XMLHttpRequest used for upload, - since there is no progress to observe) - And on success (204) the action removes the image node from - ArticleEditor's content via a new exposed removeImage(imageId) method, - which moves useArticleDraft's state to "dirty" the same way any other - body edit does; ConfirmDialog then closes and a toast appears: color - success, icon i-lucide-circle-check, title "Image removed", - description "Save to keep this change." - And on any failure (404 for either the article or the image, 500, or a - network failure) the action throws, so ConfirmDialog shows its own - inline error "Couldn't remove the image, so it's still there. Try - again in a moment." and stays open; the image node is not removed and - the draft's dirty state is unchanged - And a 401 is handled entirely by useApi's global handling - (features/api_client_global_error_handling.feature) and is out of - scope for new scenarios here - And once the confirm dialog is open, its cancel/confirm/failure - behaviour (covered below) is identical regardless of which trigger - opened it, so those scenarios are written once, against the button - trigger, and are not duplicated per trigger + And EditorView reacts to "request-remove-image" by opening ConfirmDialog with tone "neutral", + title "Remove this image from the article?", description "It's removed from this article + only. The file stays in your library and in any other article that uses it.", confirmLabel + "Remove image" and cancelLabel "Cancel"; the confirm button is primary, not red + And the body of the confirm is a bordered row with a 56 × 42 px thumbnail of the image, its + filename, and "Stays in the library · {size}", for example "north-wall.jpg" and "Stays in + the library · 3.9 MB" + And the filename and size come from the images the SPA has seen this session, kept by image + id: those inserted from either tab and those loaded in the Library list + And confirming runs removeImage(imageId), moves useArticleDraft to "dirty", closes the + dialog and shows the toast; the action cannot fail, so there is no inline error + And a thumbnail uses the image's src from the node - Scenario: Selecting an inserted image reveals its delete button - Given the user is on /article/12/edit with an image in the body carrying - data-image-id "10" + Scenario: Selecting an inserted image reveals "Remove image" + Given the user is on /article/12/edit with an image in the body carrying data-image-id "1" When the user clicks that image - Then a delete button (trash icon, error color) appears at the image's - top-right corner + Then a button "Remove image" with a trash-2 icon shows at the image's top-right corner When the user clicks elsewhere in the editor - Then the delete button is no longer shown + Then the button is no longer shown - Scenario Outline: Either trigger opens the confirm dialog and sends no request yet, without letting Tiptap delete the node itself - Given the image with data-image-id "10" is selected on /article/12/edit + Scenario: An image without data-image-id has no button + Given the body has an image with no data-image-id + When the user clicks it + Then no "Remove image" button shows and Backspace does nothing special to it + + Scenario Outline: Either trigger opens the neutral confirm and sends nothing + Given the image with data-image-id "1" ("north-wall.jpg", 3.9 MB, inserted this session) is selected on /article/12/edit When the user - Then a dialog titled "Remove this image?" opens, with description "This - image will be removed from the article. This can't be undone.", - cancel "Cancel" and confirm "Remove image" - And no request has been sent to DELETE /v1/article/12/image/10 - And the image node is still present in the editor's content (Tiptap's - native node deletion did not run) + Then a dialog titled "Remove this image from the article?" opens, with the description "It's removed from this article only. The file stays in your library and in any other article that uses it." + And its body shows the thumbnail, "north-wall.jpg" and "Stays in the library · 3.9 MB" + And its buttons read "Cancel" and a primary "Remove image" + And no request has been sent + And the image node is still in the editor's content (Tiptap's native deletion did not run) Examples: - | triggers-removal | - | activates the image's delete button | - | presses Backspace with the image node selected | + | triggers-removal | + | activates the "Remove image" button | + | presses Backspace with the image node selected | | presses Delete with the image node selected | - Scenario: Cancelling the confirm dialog changes nothing - Given the confirm dialog is open for image id 10 on /article/12/edit + Scenario: The confirm for an image the session has not seen + Given the image with data-image-id "7" was in the saved body and is not in the session's record + When the user activates "Remove image" + Then the body row shows the thumbnail and "Stays in the library" with no filename and no size + + Scenario: Cancelling changes nothing + Given the confirm is open for image id 1 When the user activates "Cancel" - Then the dialog closes - And no request has been sent to DELETE /v1/article/12/image/10 - And the image is still present in the editor's content - And the draft's dirty state is unchanged + Then the dialog closes, no request has been sent, the image is still in the content, and the draft's dirty state is unchanged - Scenario: Confirming removes the image, marks the draft dirty, and shows a success toast - Given the confirm dialog is open for image id 10 on /article/12/edit, - the draft in the Saved state + Scenario: Confirming removes the image from the body only + Given the confirm is open for image id 1 on /article/12/edit, the draft in the Saved state When the user activates "Remove image" - Then DELETE /v1/article/12/image/10 is sent - When that request resolves 204 - Then the dialog closes - And the image is removed from the editor's content - And SaveStatus reads "Unsaved changes" and the Save button becomes - enabled - And a toast appears: color success, icon i-lucide-circle-check, title - "Image removed", description "Save to keep this change." + Then the dialog closes and the image leaves the editor's content + And SaveStatus reads "Unsaved changes" and Save is enabled + And a toast appears: color success, icon i-lucide-circle-check, title "Image removed", description "It's still in your library." + And no request of any kind has been sent - Scenario Outline: A failed removal keeps the image and shows the dialog's inline error - Given the confirm dialog is open for image id 10 on /article/12/edit + Scenario: A removal started by Backspace behaves like one started by the button + Given the confirm is open for image id 1, opened by Backspace, the draft in the Saved state When the user activates "Remove image" - And DELETE /v1/article/12/image/10 - Then the dialog stays open, showing the inline error "Couldn't remove - the image, so it's still there. Try again in a moment." - And the image is still present in the editor's content - And the draft's dirty state is unchanged + Then the outcome is the same as above - Examples: - | failure | - | resolves with a 404 response | - | resolves with a 500 response | - | never reaches the server | - - Scenario: Confirming a removal started by Backspace/Delete behaves exactly like confirming one started by the button - Given the confirm dialog is open for image id 10 on /article/12/edit, - opened by pressing Backspace with the image node selected, the draft - in the Saved state - When the user activates "Remove image" - Then DELETE /v1/article/12/image/10 is sent - When that request resolves 204 - Then the dialog closes - And the image is removed from the editor's content - And SaveStatus reads "Unsaved changes" and the Save button becomes - enabled - And a toast appears: color success, icon i-lucide-circle-check, title - "Image removed", description "Save to keep this change." + Scenario: The removed image can be picked again + Given image id 1 was removed from the body this session + When the user opens the Library tab + Then "north-wall.jpg" no longer carries "In this article" diff --git a/features/editor_image_removal.qa.md b/features/editor_image_removal.qa.md index 5e2fa90..5f66ef2 100644 --- a/features/editor_image_removal.qa.md +++ b/features/editor_image_removal.qa.md @@ -1,88 +1,25 @@ -# QA: Removing an inline image from the editor +# QA: Removing an inline image from the article ## Automated (Vitest + Vue Test Utils, `task test:web`) -Mount EditorPage inside DashboardShell at `/article/12/edit` with the -editor_core.qa.md fixture article (id 12, "How we price a bathroom -renovation", is_online false), GET already resolved 200, and a body -containing ``. Mock `useApi`'s `request` for `DELETE /v1/article/12/image/10` -(fetch-based, not XHR — reuse whatever mock -`api_client_global_error_handling.qa.md` already sets up for `request`). +Mount EditorPage at `/article/12/edit` with the `editor_core.qa.md` fixture article and a body of ``. Seed the session's image record with id 1 ("north-wall.jpg", 3.9 MB). Spy on `request` and on `XMLHttpRequest` and assert they are never called by any step below. -1. **Selecting the image reveals the delete button** — click the image - node; assert a delete button (trash icon, error color) renders anchored - to it; click elsewhere in the editor; assert the button is gone. -2. **Activating the delete button opens the confirm dialog** — select the - image, activate its delete button; assert a dialog titled "Remove this - image?", description "This image will be removed from the article. This - can't be undone.", cancel "Cancel", confirm "Remove image"; assert no - request was sent yet; assert the image node is still present (Tiptap's - native deletion did not run). -3. **Backspace on the selected image opens the same confirm dialog instead - of deleting the node** — select the image node (NodeSelection), fire a - `keydown` for `Backspace`; assert the same dialog as step 2 opens; - assert no request was sent yet; assert the image node is still present - in the editor's document (i.e. the keydown was intercepted before - Tiptap's default node-deletion ran, not just undone afterwards). -4. **Delete on the selected image opens the same confirm dialog instead of - deleting the node** — same as step 3 but firing `keydown` for `Delete`. -5. **Cancelling changes nothing** — from the open dialog (opened via the - button), click "Cancel"; assert the dialog closes, no request was sent, - the image node is still in the editor's content, and SaveStatus/dirty - state is unchanged. -6. **Confirming removes the image, marks dirty, and toasts (button - trigger)** — from the open dialog (Saved state), click "Remove image"; - assert a DELETE request to `/v1/article/12/image/10` was made; resolve - it 204; assert the dialog closes, the image node is gone from the - editor's content, SaveStatus reads "Unsaved changes" and Save is - enabled, and a toast appears with color success, icon - `i-lucide-circle-check`, title "Image removed", description "Save to - keep this change." -7. **Confirming removes the image the same way when opened via - Backspace/Delete** — repeat step 6's assertions, but open the dialog via - the Backspace keydown from step 3 instead of the button; assert the - outcome (request, node removal, dirty state, toast) is identical. -8. **A failed removal keeps the image and shows the inline error, - Examples-driven** — for a 404 response, a 500 response, and a rejected/ - network-failure request: from the open dialog (button trigger), click - "Remove image"; resolve/reject accordingly; assert the dialog stays open - showing the inline error "Couldn't remove the image, so it's still - there. Try again in a moment."; assert the image node is still present - and the draft's dirty state is unchanged. +1. **Selecting reveals the button** - click the image; assert a button named "Remove image" with a trash-2 icon renders at the corner; click elsewhere; assert it is gone. +2. **No id, no button** - an image without `data-image-id`: no button, and a Backspace keydown does not open the dialog. +3. **Triggers, Examples-driven** - button click, Backspace keydown and Delete keydown each open the dialog with the exact title, description, "Cancel" and a primary (not error) "Remove image"; assert the body shows the thumbnail, "north-wall.jpg" and "Stays in the library · 3.9 MB"; assert no request; assert the node is still in the document. +4. **Unseen image** - id 7 not in the record; assert the row shows the thumbnail and "Stays in the library" only. +5. **Cancel changes nothing.** +6. **Confirm** - click "Remove image"; assert the dialog closes, the node is gone, SaveStatus "Unsaved changes", Save enabled, the toast (color success, icon `i-lucide-circle-check`, "Image removed" / "It's still in your library."), and zero requests. +7. **Backspace-opened confirm** gives the same outcome as 6. +8. **Removed image loses its tag** - open the Library after step 6; assert "north-wall.jpg" has no "In this article" tag. -## Visual +## Visual: `task visual:check -- media-picker` -No `docs/design/` board or state exists for this trigger (see the feature's -Background). Per the user's explicit direction this slice ships the SPA -behaviour anyway, but `task visual:check -- editor` has nothing to compare -it against, so no visual capture is added for it — this is an intentional -gap, `skip: no-design-coverage` (distinct from `skip: clock-sensitive`), not -a QA oversight. If a board for this state is added to `docs/design/` later, -it needs its own follow-up slice to add the visual check. +The states "Remove image: confirm" and "Removed" are in the `media-picker` group; the setups are in `editor_media_picker.qa.md`. The sample data is north-wall.jpg, image id 1, 3.9 MB. The "Remove image" button is visible in "Remove image: confirm" and "Inserted", so the setup must select the image node. The confirm's confirm button must not be error-coloured. -- [ ] Keyboard/focus: once a delete button is shown, it is reachable by Tab - and activates on Enter/Space; pressing Backspace/Delete with the - image node selected but focus elsewhere in the page does not trigger - removal (only an actual Tiptap NodeSelection on the image should); - ConfirmDialog's existing focus trap and cancel-button-default-focus - behaviour (already covered by the article-delete flow) applies - unchanged regardless of which trigger opened it. +- [ ] Keyboard and focus: the button is reachable by Tab and activates with Enter or Space. Backspace or Delete with focus outside the editor does nothing. ConfirmDialog's focus trap and default focus on "Cancel" are unchanged. ## Manual verification -- **Real DELETE round trip against a running server** — Vitest only mocks - `useApi`'s request; confirm a real removal (204/404/500) against - `task dev` with a live SQLite file and the real - `DELETE /v1/article/{id}/image/{imageId}` handler, since - `features/remove_article_image.feature`'s httptest suite covers the - handler in isolation but never through this SPA flow. -- **Real keyboard behaviour** — confirm in a real browser that selecting an - image (click) and pressing Backspace/Delete opens the confirm dialog - rather than deleting the node, including with an IME or on a platform - where Backspace/Delete semantics differ, since jsdom's `keydown` - simulation doesn't exercise Tiptap's real ProseMirror keymap precedence. -- **Design review of the delete-button/ConfirmDialog approach** — since - there is no board for this state, once a board is added, re-verify this - implementation still matches it (or update the implementation to match); - until then this is a best-effort, design-team-unreviewed UI. +- **Real keyboard behaviour** in a real browser, including an IME: Backspace or Delete on a selected image opens the confirm instead of deleting the node (jsdom doesn't exercise ProseMirror's keymap precedence). +- **Save after removal** against `task dev`: remove an image, save, reload; the image is gone from the article and still listed in `/media`. diff --git a/features/editor_image_upload.feature b/features/editor_image_upload.feature index 74f3cbc..a20baa0 100644 --- a/features/editor_image_upload.feature +++ b/features/editor_image_upload.feature @@ -1,136 +1,191 @@ -Feature: Wiring the editor's image upload flow +Feature: The editor's Upload tab uploads to the library As a verified RenewCMS owner writing an article - I want the editor's image button (and dropping a file onto the editor) to open an upload - modal that validates the file, shows progress, and inserts the result at my cursor + I want the Upload tab of the "Add an image" dialog to check my file, spot duplicates, + upload to my library and insert the result at my cursor So that I can add images to an article's body without leaving the editor, with clear feedback when a file is rejected or the upload fails Background: - Given EditorView.vue listens for ArticleEditor's "request-image" event and, on it, opens - ImageUploader (a UModal titled "Add an image") with the current article's id - And dropping a file directly onto the editor surface also opens ImageUploader, with that - file already provided to it instead of an empty idle state - And ImageUploader validates a chosen or dropped file in the browser, checking both its MIME - type and its file extension against png/jpeg/webp/svg, and that its size is at most - 10 × 1024 × 1024 bytes, before sending anything - And a file that passes validation is uploaded via a new useImageUpload composable, using - XMLHttpRequest (not useApi) so upload progress is observable, as multipart/form-data with - the file under the field name "file", to POST /v1/article/{articleId}/image, per - features/upload_article_image.feature's already-implemented and already-mounted contract - And a 201 response's {"location": "..."} is inserted into ArticleEditor's content at the - current cursor position via an exposed insertImage(src, alt) method, with alt ""; this - moves useArticleDraft's state to "dirty" the same way any other body edit does - And closing the modal while an upload is in flight aborts the XMLHttpRequest - And a 401 response is handled entirely by useApi's global handling - (features/api_client_global_error_handling.feature) and is out of scope for new scenarios - here; the same is true of a 404 (the article was deleted elsewhere), which the editor's - existing not-found handling already covers (features/editor_core.feature) + Given this slice is SPA only for this file: the API from the media library slice is used + unchanged + And EditorView listens for ArticleEditor's "request-image" event and opens ImageUploader + (a UModal titled "Add an image") with the picker tabs, whose Library tab is specified in + features/editor_media_picker.feature + And dropping a file onto the editor surface opens ImageUploader on the Upload tab with that + file already provided, and no list request is sent until the Library tab is opened + And this file supersedes these statements of the earlier version: the upload goes to + POST /v1/media/ and no longer to POST /v1/article/{articleId}/image; the 201 body is the + library item { id, filename, url, size, content_type, width, height, sha256, uploaded_by, + created_at, used_in, duplicate }; there is no article id in any upload request and no 404 + for a deleted article on upload + And ImageUploader validates a chosen or dropped file in the browser with validateImageFile + from web/src/lib/imageUpload.ts, size first (10485760 bytes allowed, one byte more is too + large), then MIME type and extension must both be allowed (png, jpeg, webp, svg), before + hashing or sending anything + And a file that passes validation is hashed with hashFile from web/src/lib/fileHash.ts (the + lib shared with MediaPage), and GET /v1/media/?hash= is sent through useApi; + when hashFile resolves null, or the lookup fails with 500 or a network error, the check + is skipped and the flow continues as if nothing matched + And a file with no match continues exactly as in features/editor_image_crop_resize.feature: + a JPEG, PNG or WEBP opens the crop and resize step, an SVG is uploaded immediately + And the upload is sent with XMLHttpRequest (not useApi) so progress is observable, as + multipart/form-data with the file under the field name "file" to POST /v1/media/, with the + jwt cookie + And a 201 inserts the item's url at the cursor through insertImage(item.url, "", item.id), + so the node carries data-image-id, and useArticleDraft moves to "dirty" + And closing the dialog while an upload is in flight aborts the XMLHttpRequest + And the tabs show only on the choose step (idle, the duplicate card and the Library tab); + the crop, uploading, rejected and failed steps keep the earlier look without tabs + And the footer on the Upload tab holds only the step's own actions ("Cancel" in idle); + there is no "Insert image" button on it - Scenario: Opening the uploader from the toolbar's image button shows the idle state - Given the user is on /article/12/edit with the editor focused - When the user activates the toolbar's "Image" button - Then ImageUploader opens titled "Add an image" - And it shows "Drop an image here", a "Choose file" action, and the help text - "PNG, JPEG, WEBP, SVG · max 10 MB" - And its cancel action reads "Cancel" - And no request has been sent + Rule: Opening - Scenario: Dragging a file over the editor surface shows the dragging-over state - Given ImageUploader is open in the idle state - When the user drags a file over the drop zone - Then the drop zone reads "Drop to upload" + Scenario: The toolbar's image button opens on Upload when the library is empty + Given the user is on /article/12/edit with the editor focused + And GET /v1/media/?limit=24 resolves 200 with no item + When the user activates the toolbar's "Image" button + Then ImageUploader opens titled "Add an image" with the tabs "Upload" (icon upload) and "Library" (icon images) + And the Upload tab is selected and shows the image-plus icon, "Drop an image here, or choose a file" and "PNG, JPEG, WEBP, SVG · max 10 MB" + And its footer reads "Cancel" only + And no POST has been sent - Scenario: Dropping a valid file onto the editor opens the uploader already uploading it - Given the user is on /article/12/edit with the editor focused and ImageUploader is not open - When the user drops a file named "before.png", content type "image/png", size 3.9 MB, onto - the editor surface - Then ImageUploader opens titled "Add an image" - And POST /v1/article/12/image is sent with that file under the field name "file" - And the uploading state is shown, not the idle state + Scenario: Dropping a file opens Upload with the file + Given the user is on /article/12/edit and ImageUploader is not open + When the user drops a file named "before.png", content type "image/png", size 3.9 MB, onto the editor surface + Then ImageUploader opens on the Upload tab + And GET /v1/media/?hash= is sent, and no request to GET /v1/media/?limit=24 is sent + And when the lookup resolves 200 with no item the crop and resize step shows for "before.png" + And no POST /v1/media/ has been sent - Scenario Outline: Choosing a file the browser can reject never sends a request - Given ImageUploader is open in the idle state for article 12 - When the user chooses a file named "", content type "", size - Then no request is sent to POST /v1/article/12/image - And the rejected state shows the title "\"\" can't be used" - And its text reads "" - And its confirm action reads "Choose another file" and its cancel action reads "Cancel" + Scenario: Dragging a file over the drop zone shows the dragging-over state + Given ImageUploader is open on the Upload tab in the idle state + When the user drags a file over the drop zone + Then the drop zone reads "Drop to upload" - Examples: - | filename | content_type | size | description | - | plan.pdf | application/pdf | 1.2 MB | Use a PNG, JPEG, WEBP or SVG image, up to 10 MB. | - | disguised.png | application/pdf | 1.2 MB | Use a PNG, JPEG, WEBP or SVG image, up to 10 MB. | - | north-wall.tif | image/png | 14.2 MB | north-wall.tif is 14.2 MB. Images can be up to 10 MB. | + Rule: Browser checks, nothing is sent - Scenario: "Choose another file" from the rejected state returns to idle - Given ImageUploader is in the rejected state for a file named "plan.pdf" - When the user activates "Choose another file" - Then the idle state is shown again, with the file picker available + Scenario Outline: Choosing a file the browser can reject sends nothing + Given ImageUploader is open in the idle state + When the user chooses a file named "", content type "", size + Then no hash lookup and no POST /v1/media/ is sent + And the rejected state shows the title "\"\" can't be used" + And its text reads "" + And its confirm action reads "Choose another file" and its cancel action reads "Cancel" - Scenario: A valid upload shows progress and its cancel action reads "Cancel upload" - Given ImageUploader is open in the idle state for article 12 - When the user chooses a file named "north-wall.jpg", content type "image/jpeg", size 3.9 MB - Then POST /v1/article/12/image is sent with that file under the field name "file" - And the uploading state shows "north-wall.jpg 0%" and "Uploading 0.0 of 3.9 MB" - And its cancel action reads "Cancel upload" - When the upload reports 2.4 MB sent of 3.9 MB - Then the uploading state shows "north-wall.jpg 62%" and "Uploading 2.4 of 3.9 MB" + Examples: + | filename | content_type | size | description | + | plan.pdf | application/pdf | 1.2 MB | Use a PNG, JPEG, WEBP or SVG image, up to 10 MB. | + | disguised.png | application/pdf | 1.2 MB | Use a PNG, JPEG, WEBP or SVG image, up to 10 MB. | + | north-wall.tif | image/png | 14.2 MB | north-wall.tif is 14.2 MB. Images can be up to 10 MB. | - Scenario: Closing the dialog during an upload cancels it - Given ImageUploader is uploading a file named "north-wall.jpg" to article 12 - When the user activates "Cancel upload" (or otherwise closes the modal) - Then the in-flight XMLHttpRequest is aborted - And the modal closes - And the editor's body is unchanged and remains in its previous dirty/clean state + Scenario: "Choose another file" from the rejected state returns to idle + Given ImageUploader is in the rejected state for "plan.pdf" + When the user activates "Choose another file" + Then the idle state is shown again, with the tabs - Scenario: A successful upload inserts the image at the cursor and marks the draft dirty - Given the user is on /article/12/edit in the Saved state and ImageUploader is uploading a - file named "north-wall.jpg" to article 12 - When POST /v1/article/12/image resolves 201 with {"location": "/static/uploadedImages/north-wall.jpg"} - Then the uploading state briefly shows "Image added" - And the image at "/static/uploadedImages/north-wall.jpg" is inserted into the editor's - content at the cursor via insertImage - And SaveStatus reads "Unsaved changes" and the Save button becomes enabled - And a toast appears: color success, icon i-lucide-circle-check, title "Image added", - description "Save to keep it in the article." - And the modal closes 600 ms after the response resolves + Rule: Already in your library - Scenario Outline: A server-side upload failure keeps the article unchanged and offers a retry - Given ImageUploader is uploading a file named "north-wall.jpg" to article 12 - When POST /v1/article/12/image - Then the failed state shows the title "Upload failed" - And its text reads "The server didn't accept the file. Your article hasn't changed." - And its confirm action reads "Try again" - And the editor's body is unchanged, and no image is inserted + Scenario: A file whose content exists shows the duplicate card and sends nothing + Given ImageUploader is open in the idle state + And GET /v1/media/?hash= resolves 200 with the image "north-wall.jpg" (id 1, 4032 × 3024, uploaded 12 Sep 2026 by Marta Alves) + When the user chooses a valid file named "north-wall.jpg" of 3.9 MB + Then the Upload tab shows a card with an info border: a 72 × 54 thumbnail of the existing image, the title "Already in your library" and the text "north-wall.jpg matches an image uploaded on 12 Sep 2026 by Marta Alves." + And the card has the buttons "Use existing" (primary) and "Upload anyway", stacked and full width below md + And no POST /v1/media/ has been sent and the crop step has not opened - Examples: - | failure | - | resolves with a 500 response | - | never reaches the server | + Scenario: Several matches name the oldest one + Given the hash lookup resolves 200 with several images + Then the card names the oldest match - Scenario: "Try again" re-attempts the upload with the same file - Given ImageUploader is in the failed state for a file named "north-wall.jpg", article 12, - after a 500 response - When the user activates "Try again" - Then POST /v1/article/12/image is sent again with that same file under the field name "file" - And the uploading state is shown + Scenario: A match without a known uploader drops the "by" part + Given the matched image's uploaded_by is empty + Then the text reads "north-wall.jpg matches an image uploaded on 12 Sep 2026." - Scenario: A server 400 for a type the client should have caught shows the same rejected copy - Given ImageUploader is uploading a file named "notes.pdf" to article 12 (a type that - defensively passed the browser's own check, e.g. a MIME type the browser misreported) - When POST /v1/article/12/image resolves 400 - Then the rejected state shows the title "\"notes.pdf\" can't be used" - And its text reads "Use a PNG, JPEG, WEBP or SVG image, up to 10 MB." - And its confirm action reads "Choose another file" - And the editor's body is unchanged, and no image is inserted + Scenario: "Use existing" goes to the alt text step for the existing image + Given the duplicate card is showing for image id 1 + When the user activates "Use existing" + Then the Library tab is selected with "north-wall.jpg" selected, its panel visible with the empty "Alt text (optional)" field, and "Insert image" enabled + And no POST /v1/media/ has been sent - Scenario: A server 400 for a size the client should have caught shows the same size-named copy - Given ImageUploader is uploading a file named "north-wall.tif" to article 12 (a file that - defensively passed the browser's own size check somehow, e.g. its reported size changed - between selection and send) - When POST /v1/article/12/image resolves 400 - Then the rejected state shows the title "\"north-wall.tif\" can't be used" - And its text reads "north-wall.tif is 14.2 MB. Images can be up to 10 MB." - And its confirm action reads "Choose another file" - And the editor's body is unchanged, and no image is inserted + Scenario: "Upload anyway" continues to crop and upload + Given the duplicate card is showing for "north-wall.jpg" (4032 × 3024) + When the user activates "Upload anyway" + Then the crop and resize step opens for that file + And the hash lookup is not repeated + And uploading from that step sends POST /v1/media/ and follows the normal states + + Scenario: "Upload anyway" for an SVG uploads immediately + Given the duplicate card is showing for "diagram.svg" + When the user activates "Upload anyway" + Then POST /v1/media/ is sent with that file + + Scenario Outline: The duplicate check is skipped when it cannot run + Given ImageUploader is open in the idle state + When the user chooses a valid file and + Then the flow continues to the crop step (or the upload for an SVG) with no card + + Examples: + | condition | + | crypto.subtle is undefined (an insecure context) | + | GET /v1/media/?hash= resolves with a 500 response | + | GET /v1/media/?hash= never reaches the server | + + Rule: Uploading + + Scenario: A valid upload shows progress and its cancel action reads "Cancel upload" + Given the crop step was confirmed (or an SVG chosen) for "north-wall.jpg", 3.9 MB + Then POST /v1/media/ is sent with the file under the field name "file" + And the uploading state shows "north-wall.jpg 0%" and "Uploading 0.0 of 3.9 MB", and its cancel action reads "Cancel upload" + When the upload reports 2.4 MB sent of 3.9 MB + Then it shows "north-wall.jpg 62%" and "Uploading 2.4 of 3.9 MB" + + Scenario: Closing the dialog during an upload cancels it + Given ImageUploader is uploading "north-wall.jpg" + When the user activates "Cancel upload" (or otherwise closes the dialog) + Then the in-flight XMLHttpRequest is aborted and the dialog closes + And the editor's body and its dirty or clean state are unchanged + + Scenario: A successful upload inserts the image at the cursor and marks the draft dirty + Given the user is on /article/12/edit in the Saved state and "north-wall.jpg" is uploading + When POST /v1/media/ resolves 201 with the item { id: 13, url: "/static/uploadedImages/13.jpg", filename: "north-wall.jpg" } + Then the uploading state briefly shows "Image added" + And the image "/static/uploadedImages/13.jpg" is inserted at the cursor with alt "" and data-image-id 13 + And SaveStatus reads "Unsaved changes" and Save is enabled + And a toast appears: color success, icon i-lucide-circle-check, title "Image added", description "Save to keep it in the article." + And the dialog closes 600 ms after the response + + Scenario Outline: A server answer for a file that passed the browser checks + Given ImageUploader is uploading "" of + When POST /v1/media/ resolves + Then the state shows the title "" + And its text reads "<text>" + And its confirm action reads "<confirm>" + And no image is inserted and the editor's body is unchanged + + Examples: + | file | size | status | state | title | text | confirm | + | notes.pdf | 210 KB | 415 | rejected | "notes.pdf" can't be used | Use a PNG, JPEG, WEBP or SVG image, up to 10 MB. | Choose another file | + | north-wall.tif | 14.2 MB | 413 | rejected | "north-wall.tif" can't be used | north-wall.tif is 14.2 MB. Images can be up to 10 MB. | Choose another file | + | broken.png | 3.0 MB | 400 | rejected | "broken.png" can't be used | Use a PNG, JPEG, WEBP or SVG image, up to 10 MB. | Choose another file | + | north-wall.jpg | 3.9 MB | 500 | failed | Upload failed | The server didn't accept the file. Your article hasn't changed. | Try again | + | north-wall.jpg | 3.9 MB | 429 | failed | Upload failed | The server didn't accept the file. Your article hasn't changed. | Try again | + | north-wall.jpg | 3.9 MB | none (network) | failed | Upload failed | The server didn't accept the file. Your article hasn't changed. | Try again | + + Scenario: "Try again" re-attempts the upload with the same file + Given ImageUploader is in the failed state for "north-wall.jpg" after a 500 + When the user activates "Try again" + Then POST /v1/media/ is sent again with that same file and no new hash lookup, and the uploading state shows + + Rule: Sessions + + Scenario Outline: Session errors are global + When <call> resolves <status> + Then the user is sent where useApi sends a <status> from /article/12/edit, and no local copy shows + + Examples: + | call | status | + | POST /v1/media/ (XHR) | 401 | + | POST /v1/media/ (XHR) | 403 | + | GET /v1/media/?hash= | 401 | diff --git a/features/editor_image_upload.qa.md b/features/editor_image_upload.qa.md index 9531f3d..473645d 100644 --- a/features/editor_image_upload.qa.md +++ b/features/editor_image_upload.qa.md @@ -1,90 +1,33 @@ -# QA: Wiring the editor's image upload flow +# QA: The editor's Upload tab uploads to the library ## Automated (Vitest + Vue Test Utils, `task test:web`) -Mock `XMLHttpRequest` (a fake implementing `open`/`setRequestHeader`/`send`/`abort` and firing -`upload.progress`, `load`, `error`) for `POST /v1/article/{id}/image`; useApi/fetch is not -involved in this slice. Mount EditorPage inside DashboardShell at `/article/12/edit` with the -editor_core.qa.md fixture article (id 12, title "How we price a bathroom renovation", is_online -false), GET already resolved 200. +Mock `XMLHttpRequest` (a fake with `open`, `setRequestHeader`, `send`, `abort`, firing `upload.progress`, `load`, `error`) for `POST /v1/media/`. Mock useApi's `request` for `GET /v1/media/` (list and `?hash=`), reusing the mock from `api_client_global_error_handling.qa.md`. Stub `hashFile` where a hash is needed, and add one test with `crypto.subtle` deleted. Mount EditorPage inside DashboardShell at `/article/12/edit` with the `editor_core.qa.md` fixture article, GET already resolved 200. Assert URLs by parsing the query, not by string equality. -1. **Toolbar button opens the idle state** — click the toolbar's "Image" button; assert - ImageUploader opens titled "Add an image", showing "Drop an image here", "Choose file", - "PNG, JPEG, WEBP, SVG · max 10 MB", cancel "Cancel"; assert no XHR was created. -2. **Dragging over shows "Drop to upload"** — with the uploader open idle, fire a `dragover` on - the drop zone; assert the text changes to "Drop to upload". -3. **Dropping a file on the editor opens the uploader already uploading** — with the uploader - closed, dispatch a `drop` event with a file `before.png` (image/png, 3.9 MB) on the editor - surface; assert ImageUploader opens and an XHR to `/v1/article/12/image` is created with that - file, and the uploading state (not idle) is shown. -4. **Rejected — Examples-driven on type, extension mismatch, and size** — for each of `plan.pdf` - (application/pdf, 1.2 MB), `disguised.png` (application/pdf, 1.2 MB), and `north-wall.tif` - (image/png, 14.2 MB), choose the file via the `UFileUpload`; assert no XHR is created and the - rejected state shows the exact title/text/confirm/cancel copy from the feature's Examples. -5. **"Choose another file" returns to idle** — from the rejected state, click "Choose another - file"; assert the idle state re-renders. -6. **Uploading shows live progress and "Cancel upload"** — choose `north-wall.jpg` (image/jpeg, - 3.9 MB); assert the XHR is created with that file under field `file`; assert cancel reads - "Cancel upload"; fire an `upload.progress` event with `loaded: 2.4 * 1024 * 1024, total: 3.9 * - 1024 * 1024`; assert the text reads "north-wall.jpg 62%" and "Uploading 2.4 of 3.9 MB". -7. **Closing during an upload aborts the XHR** — from the uploading state, click "Cancel - upload"; assert the fake XHR's `abort` was called, the modal closes, and the editor's body - and dirty state are unchanged from before the upload started. -8. **A 201 inserts at the cursor and marks the draft dirty** — from the uploading state, resolve - the fake XHR with status 201 and body `{"location": "/static/uploadedImages/north-wall.jpg"}`; - assert `insertImage` (or the editor's exposed method) was called with that location; assert - SaveStatus now reads "Unsaved changes" and Save is enabled; assert a toast appears with color - success, icon `i-lucide-circle-check`, title "Image added", description "Save to keep it in - the article."; advance fake timers 600 ms and assert the modal has closed. -9. **A 500 or network failure shows "Upload failed" and keeps the article unchanged, Examples- - driven** — for (a) the fake XHR firing `load` with status 500 and (b) firing `error`; assert - the failed state shows title "Upload failed", text "The server didn't accept the file. Your - article hasn't changed.", confirm "Try again"; assert no `insertImage` call and the editor's - body/dirty state is unchanged. -10. **"Try again" resends the same file** — from the failed state for `north-wall.jpg`, click - "Try again"; assert a new XHR is created for `/v1/article/12/image` with the same file, and - the uploading state is shown again. -11. **A server 400 for a type the client should have caught shows the rejected copy** — from an - uploading state for `notes.pdf`, resolve the fake XHR with status 400; assert the rejected - state shows title "\"notes.pdf\" can't be used", text "Use a PNG, JPEG, WEBP or SVG image, - up to 10 MB.", confirm "Choose another file"; assert no `insertImage` call. -12. **A server 400 for a size the client should have caught shows the size-named rejected copy** - — from an uploading state for `north-wall.tif`, resolve the fake XHR with status 400; assert - the rejected state shows title "\"north-wall.tif\" can't be used", text "north-wall.tif is - 14.2 MB. Images can be up to 10 MB.", confirm "Choose another file"; assert no `insertImage` - call. +1. **Toolbar button, empty library** - `GET /v1/media/?limit=24` resolves `{items: [], next_cursor: null, counts: {all: 0, unused: 0, duplicates: 0}}`; click "Image"; assert the dialog "Add an image", both tabs, the Upload tab selected, the drop zone copy, footer "Cancel" only, no XHR. +2. **Drop opens Upload with the file** - dispatch `drop` with `before.png` (image/png, 3.9 MB); assert the Upload tab, a `?hash=` request, no `?limit=24` list request, then the crop step after an empty lookup, and no XHR. +3. **Dragging over shows "Drop to upload"**. +4. **Rejected, Examples-driven** - `plan.pdf`, `disguised.png`, `north-wall.tif`; assert no `?hash=` request and no XHR, plus the exact title, text, confirm and cancel. +5. **"Choose another file" returns to idle**, with the tabs back. +6. **Duplicate card** - the hash lookup resolves one item (id 1, "north-wall.jpg", `created_at` 12 Sep 2026, `uploaded_by` "Marta Alves"); assert the card copy and buttons, no XHR, and no crop step. +7. **Several matches name the oldest**, and **no uploader drops the "by" part**. +8. **"Use existing"** - assert the Library tab is selected, image 1 selected, the alt field empty, "Insert image" enabled, and no XHR. +9. **"Upload anyway"** - assert the crop step opens (raster) with no second `?hash=` request; for an SVG assert the XHR is created at once. +10. **Check skipped** - Examples: `hashFile` resolves null, lookup 500, lookup rejects; assert the crop step opens with no card. +11. **Uploading and progress** - confirm the crop step; assert the XHR is `POST /v1/media/` with field `file`, cancel "Cancel upload", and `loaded: 2.4 * 1024 * 1024, total: 3.9 * 1024 * 1024` gives "north-wall.jpg 62%" and "Uploading 2.4 of 3.9 MB". +12. **Closing during an upload aborts** - `abort` called, dialog closed, body and dirty state unchanged. +13. **201 inserts** - resolve the XHR with the item (id 13, url `/static/uploadedImages/13.jpg`); assert `insertImage` was called with (url, "", 13), SaveStatus "Unsaved changes", Save enabled, the toast (color success, icon `i-lucide-circle-check`, "Image added" / "Save to keep it in the article."), and after 600 ms of fake timers the dialog closes. +14. **Server answers, Examples-driven** - 415, 413, 400 give the rejected copy; 500, 429 and a network `error` give "Upload failed"; assert no `insertImage` call and the body unchanged. +15. **"Try again" resends the same file** with no new `?hash=` request. +16. **Sessions** - Examples from the feature; assert the same redirect `useApi` performs, with no local copy. -## Visual (manual verification — desktop and mobile, light and dark) +## Visual -Reference: `docs/design/screens/editor.md` states "Upload: choose", "Upload: in progress", -"Upload: rejected", "Upload: inserted", and `docs/design/source/RenewCMS Screens Back- -office.dc.html`. Run `task visual:check -- editor` for each state below; add each state's mock -setup to `web/visual/states.ts` first, reusing editor_core.qa.md's fixture article (id 12, title -"How we price a bathroom renovation", draft, "Today, 11:02", 3 images) plus the sample file data -from the board: filename "north-wall.jpg", size 3.9 MB, progress 62% / 2.4 of 3.9 MB uploaded. - -- [ ] Upload: choose — modal "Add an image", idle state, cancel "Cancel". -- [ ] Upload: in progress — modal "Add an image", uploading "north-wall.jpg 62%" / "Uploading - 2.4 of 3.9 MB", cancel "Cancel upload". -- [ ] Upload: rejected — modal "Add an image", rejected state for a sample rejected file per the - board's script, cancel "Cancel", confirm "Choose another file". -- [ ] Upload: inserted — toast "Image added" / "Save to keep it in the article.", editor status - "draft", save "dirty". -- [ ] Mobile (all of the above at 390×844): same modal layout constraints as other editor - modals (leading arrow-left button, trailing ellipsis in the header behind it). -- [ ] Keyboard/focus: the drop zone is reachable by Tab and Enter opens the file picker; focus - moves into the modal on open and is trapped there; Esc during "Upload: in progress" - behaves like "Cancel upload" (aborts, closes); the progress bar's `role="progressbar"` / - `aria-valuenow` updates are announced without spamming on every progress tick. - - [ ] Motion: modal open/close follows the standard 240 ms timing; the success state's 600 ms - auto-close is not visually checked as a capture (it's a timing behavior, not a static - state) — cover only via the Vitest fake-timer assertion above. +The Upload tab states are drawn in the `media-picker` group; see `editor_media_picker.qa.md`. The editor-group state 'Upload: choose' awaits the user's editor board update: the user refreshes the editor board themselves, and until then `task visual:check -- editor` is expected to fail on that state. The states "Upload: in progress" and "Upload: rejected" and the crop and resize states are unchanged. `web/visual/states.ts` line 2309 mocks `POST /v1/article/12/image` and must mock `POST /v1/media/` with the item shape, and every editor upload state needs a `GET /v1/media/?limit=24` mock. ## Manual verification -- **Real POST round trip against a running server** — Vitest only mocks `XMLHttpRequest`; - confirm a real upload (200/400/401/404/500) against `task dev` with a live SQLite file and the - real `POST /v1/article/{id}/image` handler, since `features/upload_article_image.feature`'s - httptest suite covers the handler in isolation but never through this SPA flow. -- **Real drag-and-drop from the OS file picker** — jsdom's `drop` event is synthetic; confirm a - real OS-level drag of an image file onto the editor in a real browser. +- **Real round trip against `task dev`** with a live SQLite file: a real upload through the SPA (201, 413, 415, 400, 401), then the image shows in `/media`. +- **Real OS drag and drop** of an image onto the editor in a real browser. +- **Real `crypto.subtle`** in a secure context (localhost or HTTPS), including the duplicate card for a file already in the library, and the skipped check on plain HTTP. +- **Keyboard and focus**: the drop zone is reachable by Tab and opens the file chooser with Enter or Space. Focus moves into the dialog and is trapped. Esc during "Upload: in progress" aborts and closes. Tabs are reachable and arrow keys move between them. diff --git a/features/editor_media_picker.feature b/features/editor_media_picker.feature new file mode 100644 index 0000000..a5f84bd --- /dev/null +++ b/features/editor_media_picker.feature @@ -0,0 +1,221 @@ +Feature: The editor's Library tab, picking an image from the media library + + As a verified RenewCMS owner writing an article + I want to browse my library from the "Add an image" dialog and insert an existing image + with alt text + So that I can reuse an image without uploading it again + + Background: + Given this slice is SPA only for this feature: the API from the media library slice is used + unchanged, and the Go change to remove the legacy alias is specified in + features/remove_legacy_article_image_alias.feature + And the screen is docs/design/screens/media-picker.md, drawn by docs/design/source/MediaFrame.dc.html, + inside ImageUploader (UModal "Add an image") on /article/:id/edit + And the dialog has a title "Add an image", an x to close, and two tabs under the title, + "Upload" (icon upload) and "Library" (icon images), 36 px high on desktop and 44 px on + mobile with a 2 px primary underline on the selected one; it is 640 px wide on desktop and + a bottom sheet on mobile + And the footer reads "Cancel", and on the Library tab also a primary "Insert image" that is + disabled until an image is selected + And the Library tab calls GET /v1/media/ through useApi with limit=24, with q only while a + search is set and cursor only when loading more (no filter and no sort parameter), and the + Upload tab is specified in features/editor_image_upload.feature + And the response is { items, next_cursor, counts }, newest first; sizes read "8 KB" and + "3.9 MB", dimensions read "2400 × 1600" (an SVG reads "Vector"), dates are absolute + And the 12-image fixture, ids 1 to 12, is the one in features/media_browse.feature, and the + picker mocks use next_cursor "c24" and counts { all: 48, unused: 2, duplicates: 2 } + And an image is "in this article" when an image node with its id as data-image-id is in + the editor's current content, unsaved insertions included, when the dialog opens; + in the examples below images 1 (north-wall.jpg) and 4 (logo-renew.svg) are in the article + And the search field reads "Search by filename" (32 px high on desktop, 44 px with 16 px + text on mobile), with a search icon and, once it has text, an x that clears it + And the grid has 4 columns on desktop and 3 on mobile with a 12 px gap (8 px between + columns on mobile), square tiles with a 1 px border, and the filename under each tile in + 12 px toned text with an ellipsis; it scrolls inside the dialog, 300 px high with nothing + selected, 190 px on desktop and 120 px on mobile with one selected + And every tile is one button whose accessible name reads like the Media page's, for example + "north-wall.jpg, 3.9 MB, used in 2 articles" or "window-frame.jpg, 1.8 MB, unused" + And skeleton tiles show only after 150 ms: 8 on desktop and 9 on mobile for a first load + or a search, one row (4 or 3) at the end for "Load more" + And a 401 or 403 on any of these calls is handled globally by useApi, with no local copy + + Rule: Which tab opens + + Scenario: The library has images, so Library opens + Given GET /v1/media/?limit=24 resolves 200 with the 12-image fixture + When the user activates the toolbar's "Image" button + Then the Library tab is selected and the footer reads "Cancel" and a disabled "Insert image" + And nothing is drawn in the grid for the first 150 ms, then 8 skeleton tiles show until the response arrives + + Scenario: The library is empty, so Upload opens + Given GET /v1/media/?limit=24 resolves 200 with no item and no search set + When the dialog opens + Then the Upload tab is selected, in its idle state + When the user selects the Library tab + Then the empty panel shows + + Scenario: A dropped file opens Upload and sends no list request + When the user drops "before.png" onto the editor + Then the Upload tab is selected and no GET /v1/media/?limit=24 is sent + When the user selects the Library tab + Then GET /v1/media/?limit=24 is sent + + Rule: Browsing + + Scenario: Library, nothing selected + Given the first page resolved with the 12-image fixture + Then the line "Newest first · 48 images" shows under the search field + And 12 tiles show in fixture order, 4 per row on desktop and 3 on mobile, each with its filename + And the tiles of "north-wall.jpg" and "logo-renew.svg" carry a sand "In this article" tag (bottom-left, 18 px high, 11 px text) + And no other tile carries it + And "Load more" shows below the grid, and "Insert image" is disabled + + Scenario: A search is active, so the count is not shown + Given the user typed "wall" and the response resolved with 2 items + Then the line reads "Newest first" + + Scenario: Load more is hidden on the last page + Given the response has next_cursor null + Then "Load more" is not shown + + Scenario Outline: A tile's accessible name + Given the first page resolved with the 12-image fixture + Then the tile for "<filename>" is named "<name>" + + Examples: + | filename | name | + | north-wall.jpg | north-wall.jpg, 3.9 MB, used in 2 articles | + | kitchen-before.webp | kitchen-before.webp, 1.1 MB, used in 1 article | + | window-frame.jpg | window-frame.jpg, 1.8 MB, unused | + + Rule: Selection + + Scenario Outline: A tile is selected by pointer or keyboard + Given the first page resolved with the 12-image fixture + When the user <action> on the tile of "kitchen-after.webp" + Then that tile has a 2 px primary outline (2 px offset) and a 20 px primary check at its top-right + And "Insert image" is enabled + + Examples: + | action | + | clicks | + | presses Enter | + | presses Space | + + Scenario: The selection panel + Given "kitchen-after.webp" is selected + Then the grid shrinks and a panel shows below it, under a 1 px top border + And the panel has a 56 px thumbnail, the filename "kitchen-after.webp", "2400 × 1600 · 1.2 MB", and a field "Alt text (optional)" + And the field's placeholder reads "e.g. North wall of the kitchen before plastering" + And its help reads "Describes the image for people who use a screen reader, and shows if it doesn't load. Leave it empty if the image is only decoration." + And "Load more" is hidden while an image is selected + + Scenario: Selection is single + Given "kitchen-after.webp" is selected and the alt text reads "A kitchen" + When the user selects "window-frame.jpg" + Then only "window-frame.jpg" is selected and the alt text field is empty + + Scenario: Activating the selected tile again keeps it selected + Given "kitchen-after.webp" is selected + When the user activates its tile + Then it stays selected + + Scenario: An SVG's panel reads "Vector" + When the user selects "logo-renew.svg" + Then the panel reads "Vector · 8 KB" + + Scenario: An image already in the article can be selected and inserted again + When the user selects "north-wall.jpg", which carries "In this article" + Then "Insert image" is enabled + + Rule: Insert + + Scenario: Insert with alt text + Given the user is on /article/12/edit in the Saved state with "kitchen-after.webp" (id 2, url "/static/uploadedImages/2.webp") selected + And the alt text reads "A kitchen with new oak worktops and white tiles" + When the user activates "Insert image" + Then insertImage is called with "/static/uploadedImages/2.webp", "A kitchen with new oak worktops and white tiles" and image id 2 + And the image is in the editor's content at the cursor, with data-image-id 2 + And SaveStatus reads "Unsaved changes" and Save is enabled + And a toast appears: color success, icon i-lucide-circle-check, title "Image added", description "Save to keep it in the article." + And the dialog closes at once + And no POST, PATCH or DELETE has been sent + + Scenario: Empty alt text inserts a decorative image + Given "kitchen-after.webp" is selected and the alt text is empty + When the user activates "Insert image" + Then insertImage is called with alt "" + + Scenario: Alt text lives only in the article + Given "kitchen-after.webp" is selected with alt text "A kitchen" + When the user activates "Insert image" + Then no request carries the alt text and the library item is unchanged + + Scenario Outline: Leaving without inserting changes nothing + Given "kitchen-after.webp" is selected with alt text "A kitchen" + When the user <action> + Then the dialog closes, the editor's content and dirty state are unchanged, and no toast shows + + Examples: + | action | + | activates "Cancel" | + | activates the x | + | presses Escape | + + Rule: Search + + Scenario: Typing searches after a debounce + Given the Library tab shows the 12-image fixture + When the user types "wall" + Then after a 120 ms debounce GET /v1/media/ is sent with q=wall and limit=24 and no cursor + And skeleton tiles show after 150 ms, and any selection is cleared + + Scenario: The x in the field clears the search + Given the search field holds "wall" with results shown + When the user activates the x in the field + Then the field is empty and GET /v1/media/ is sent with limit=24 and no q + + Scenario: No results + Given GET /v1/media/?q=wall&limit=24 resolves 200 with no item + Then the search icon shows with the title `No images match "wall"`, the text "Check the spelling, or search for a shorter part of the filename." and a "Clear search" button + And the search field still shows "wall" above it, and "Insert image" is disabled + When the user activates "Clear search" + Then the field is emptied and the list is requested again with no q + + Rule: Load more + + Scenario: Loading more shows progress and appends + Given the first page resolved with 12 images, next_cursor "c24" + When the user activates "Load more" + Then GET /v1/media/ is sent with limit=24 and cursor=c24 + And the button reads "Loading…" with a loader-circle and is disabled, and one row of skeleton tiles shows at the end + And focus stays on the button + When the request resolves 200 with 12 more images + Then the new tiles are appended after the first 12, and focus moves to the first new tile + + Scenario: The cursor and the search combine + Given the search "wall" is active and the page has next_cursor "c24" + When the user activates "Load more" + Then GET /v1/media/ is sent with q=wall, limit=24 and cursor=c24 + + Rule: Empty and failed + + Scenario: Empty library + Given the Library tab is selected and GET /v1/media/?limit=24 resolved with no item + Then the images icon shows with "Your library is empty", "Images you upload appear here, ready to use in any article." and a primary "Upload an image" + And "Insert image" is disabled + When the user activates "Upload an image" + Then the Upload tab is selected + + Scenario Outline: The library fails to load + Given GET /v1/media/ <failure> + When the Library tab loads + Then an error alert shows in the dialog: icon circle-x, title "Couldn't load your library", text "Check your connection and try again.", and a "Try again" button + And "Insert image" is disabled + When the user activates "Try again" + Then the same request is sent again with a skeleton, and on 200 the tiles replace the alert + + Examples: + | failure | + | resolves with a 500 response | + | never reaches the server | diff --git a/features/editor_media_picker.qa.md b/features/editor_media_picker.qa.md new file mode 100644 index 0000000..ef18449 --- /dev/null +++ b/features/editor_media_picker.qa.md @@ -0,0 +1,54 @@ +# QA: The editor's Library tab + +## Automated (Vitest + Vue Test Utils, `task test:web`) + +Mock `request` for `GET /v1/media/` by parsed query. Use the 12-image fixture from `media_browse.feature` as `items`, with `next_cursor: "c24"` and `counts: {all: 48, unused: 2, duplicates: 2}`. Mount EditorPage at `/article/12/edit` with a body containing `<img data-image-id="1">` and `<img data-image-id="4">`. Use fake timers for the 150 ms and 120 ms steps. + +1. **Opens on Library with images** - first response has items; assert Library selected and skeleton tiles only after 150 ms. +2. **Opens on Upload when empty** - empty first response; assert Upload selected; then click Library and assert the empty panel. +3. **Drop sends no list request** - drop a file; assert no `?limit=24` request until Library is clicked. +4. **Nothing selected** - assert the line, 12 tiles in order, the "In this article" tag on exactly ids 1 and 4, "Load more", "Insert image" disabled. +5. **Active search hides the count** - the line reads "Newest first". +6. **Last page** - `next_cursor: null` hides "Load more". +7. **Accessible names** - the three Examples. +8. **Selection** - click, Enter and Space each select; check, outline, "Insert image" enabled. +9. **Panel copy** - kitchen-after.webp panel, placeholder and help text exact; "Load more" hidden; SVG reads "Vector · 8 KB". +10. **Single selection resets alt**; reactivating keeps it selected. +11. **Insert** - alt "A kitchen with new oak worktops and white tiles" gives `insertImage("/static/uploadedImages/2.webp", alt, 2)`, SaveStatus "Unsaved changes", the toast, the dialog closed, no non-GET request. Empty alt gives `""`. +12. **Cancel, x and Escape** change nothing. +13. **Search** - type "wall"; at 119 ms no request, at 120 ms the request has `q=wall&limit=24` and no cursor; the selection is cleared. The field's x and "Clear search" send a request with no `q`. +14. **No results** and **Empty** copy; "Upload an image" switches to the Upload tab. +15. **Load more** - request has `cursor=c24`; button "Loading…" and disabled; skeleton row of 4 (3 below md); focus on the button, then on the first new tile after resolve; combined `q` and `cursor`. +16. **Failed to load, Examples 500 and network** - alert copy; "Try again" repeats the request; success replaces the alert. + +## Visual: `task visual:check -- media-picker` + +Add a setup in `web/visual/states.ts` for each of the 13 states in `docs/design/screens/media-picker.md`, from the board's script (`RenewCMS Screens Back-office.dc.html` lines 239 to 253). Every capture must be within 10 % at 1440×900 and 390×844, light and dark. The sample data to reuse: +- Library mock: the 12-image fixture, `counts.all` 48, `next_cursor` non-null; images 1 and 4 in the article. The dialog is opened on the editor "How we price a bathroom renovation". +- "Upload tab": Upload selected, idle drop zone. +- "Upload tab: already in your library": a chosen file named `north-wall.jpg`; the hash lookup returns id 1 (12 Sep 2026, Marta Alves); the thumbnail is 72 × 54. +- "Library: loading": skeleton tiles. +- "Library: nothing selected": the 12 tiles, tags on ids 1 and 4, "Load more". +- "Library: selected": id 2 (kitchen-after.webp, 2400 × 1600 · 1.2 MB), empty alt. +- "Library: alt text entered": id 2 with alt "A kitchen with new oak worktops and white tiles". +- "Library: loading more": 12 tiles plus one skeleton row, button "Loading…". +- "Library: empty": a mock with zero items. "Library: no results": search "wall" and zero items. +- "Library: failed to load": a 500. +- "Inserted": the editor with an image (id 1 as the board draws it) and the toast "Image added" / "Save to keep it in the article.". +- "Remove image: confirm": the confirm over the editor, image id 1 (north-wall.jpg, 3.9 MB). See `editor_image_removal.qa.md`. +- "Removed": the editor without the image and the toast "Image removed" / "It's still in your library.". + +Clock-sensitive: none. Dates are absolute. Nothing is skipped. + +Layout facts the mocks must hit (from the frame): the dialog is 640 px at 40 px from the top; the tabs row has a 20 px gap and 24 px side padding; the body has 16 px top padding and 24 px side padding; the search field is 32 px (44 px on mobile); the grid gap is 12 px (8 px between columns on mobile). + +Manual checks the screenshots can't cover: +- [ ] Focus order is tabs, search, tiles, "Load more", footer. Enter and Space select a tile. Focus returns to the toolbar's image button when the dialog closes. +- [ ] Hover on a tile and on "Load more", and the focus ring. +- [ ] Motion: the dialog follows the standard 240 ms timing. +- [ ] A real library of more than 24 images through `task dev`: "Load more" and the cursor. +- [ ] A screen reader reads the tile names and the alt text help. + +## Manual verification + +Real `GET /v1/media/` with `q` and `cursor` against `task dev` with a live SQLite file. diff --git a/features/media_library.qa.md b/features/media_library.qa.md index c77b12d..475eef2 100644 --- a/features/media_library.qa.md +++ b/features/media_library.qa.md @@ -7,7 +7,7 @@ Package: `test` (black-box). New file `test/media_library_http_test.go`. Fake ty 2. Rewrite `mockImageRepo` (`test/image_test.go`) to the new `image.Repository`: Create (stores the metadata from the ImageInput and derives a `/static/uploadedImages/<n>.<ext>` path), Get, Delete (records calls, honours `failDeleteFor`), List(query), FindByHash, Rename, ListByHash groups, Counts. Add fields for seeding `Filename`, `Size`, `ContentType`, `Width`, `Height`, `SHA256`, `UploadedByID`, `CreatedAt`. 3. Extend `mockArticleRepo` (`test/article_test.go`) with `FindByBodyContaining(filename)` (substring over `Body`) and a per-id failure switch for `UpdateBody`. Remove `AddImage` and the article `Images` field from the fake and from every test that uses them. 4. Use the existing user fake so `uploaded_by` resolves: user 1 display name "Inès Duarte", user 2 username "marta" with empty display name. -5. Wire the real use cases over the fakes: create, list, rename, delete, merge and usage media use cases, `NewCreateImageUseCase(imageRepo)`, `NewGetUserUseCase(userRepo)`. Build `media.NewHandler(...)` (`api/handlers/media`) and `image.NewHandler(...)` for the alias. +5. Wire the real use cases over the fakes: create, list, rename, delete, merge and usage media use cases, `NewCreateImageUseCase(imageRepo)`, `NewGetUserUseCase(userRepo)`. Build `media.NewHandler(...)` (`api/handlers/media`). 6. Build `jwtauth.New("HS256", []byte("test-secret"), nil)`. Mount on a fresh `chi.NewRouter()` mirroring `api/router.go`: `jwtauth.Verifier` at the top, then in the media group `Authenticator`, `RequireCurrentToken(getUser)`, `RequireVerifiedUser(getUser)`, with the six routes under `/v1/media`. 7. Mint cookies via `token.Encode(map[string]any{"user_id": uint32(n), ...})` as in the other HTTP tests. Build a stale-token cookie with a `token_version` lower than the user's, and use an unverified user for the 403 case. 8. Build multipart bodies with `mime/multipart.Writer`, using `CreatePart` with an explicit part `Content-Type` header. Generate real bytes in the test: diff --git a/features/public_article_read.feature b/features/public_article_read.feature index 718b63a..5b4e500 100644 --- a/features/public_article_read.feature +++ b/features/public_article_read.feature @@ -23,7 +23,7 @@ Feature: Single article read follows the same online/offline visibility rule as group that already holds GET /v1/article/ (features/public_article_listing.feature), so a missing or invalid jwt cookie no longer blocks the request; POST /v1/article/, DELETE /v1/article/{id}, PATCH /v1/article/{id}, PATCH /v1/article/{id}/publish, - PATCH /v1/article/{id}/unpublish and POST /v1/article/{id}/image keep requiring + and PATCH /v1/article/{id}/unpublish keep requiring Verifier + Authenticator + RequireVerifiedUser, unchanged And Handler.GetArticle reuses the existing Handler.callerIsVerified(r) helper already used by Handler.ListArticles: when the request resolves to a verified account it calls diff --git a/features/remove_legacy_article_image_alias.feature b/features/remove_legacy_article_image_alias.feature new file mode 100644 index 0000000..5def563 --- /dev/null +++ b/features/remove_legacy_article_image_alias.feature @@ -0,0 +1,42 @@ +Feature: The legacy POST /v1/article/{id}/image alias is removed + + As the maintainer of RenewCMS + I want the editor to upload through /v1/media only and the article alias deleted + So that there is one way to add an image and no dead route to keep + + Background: + Given this is a Go API change in a slice that is otherwise SPA only, made at the user's request + And features/upload_article_image.feature and its .qa.md are deleted, with their tests + And api/router.go no longer mounts r.Post("/{id}/image", ...) in NewArticleRouter, and the + imageHandler variable and the api/handlers/image import are removed from it + And the package api/handlers/image (PostLegacyArticleImage, its NewHandler and its helpers) is deleted + And CreateImageUseCase and GetArticleUseCase stay: both are used by the media handler and + the article handler, and api/pkg/upload stays because the media handler uses it + And the rest of the protected article group is unchanged, still behind jwtauth.Authenticator, + RequireCurrentToken and RequireVerifiedUser + And docs/architecture.md no longer lists api/handlers/image + And POST /v1/media/ is the only upload route, and DELETE /v1/article/{id}/image/{imageId} + stays unmounted, as before + + Scenario Outline: The removed routes are not found + Given a valid jwt cookie for verified user 1 and an article with id 1 + When the client sends <method> <path> with a valid PNG as multipart field "file" + Then the response status is 404 + And the image repository's Create is never called + And no image exists afterwards + + Examples: + | method | path | + | POST | /v1/article/1/image | + | DELETE | /v1/article/1/image/10 | + + Scenario: Uploading still works through the media route + Given a valid jwt cookie for verified user 1 + When the client sends POST /v1/media/ with a real PNG named "before.png" + Then the response status is 201 and the body is the library item with a "url" and an "id" + And GET /v1/media/ lists it + + Scenario: The rest of the article router is unchanged + Given a valid jwt cookie for verified user 1 and an article with id 1 + When the client sends PATCH /v1/article/1/publish + Then the response is as before this slice diff --git a/features/remove_legacy_article_image_alias.qa.md b/features/remove_legacy_article_image_alias.qa.md new file mode 100644 index 0000000..281e2b8 --- /dev/null +++ b/features/remove_legacy_article_image_alias.qa.md @@ -0,0 +1,21 @@ +# QA procedure: remove_legacy_article_image_alias + +Package: `test` (black-box, httptest, in-memory fakes, no live server). + +## Setup +1. Call `validator.InitValidator()`. +2. Build the article router exactly as `NewArticleRouter` does after the change (no image handler), wrapped in `Verifier`, `Authenticator`, `RequireCurrentToken` and `RequireVerifiedUser`, over fake article, image and user repositories. Reuse `newMultipartFileRequest` and `pngBytes` from `test/media_fixtures_test.go`. + +## HTTP checks +3. `POST /v1/article/1/image` with a valid PNG and a valid cookie -> 404, `Create` never called. +4. `DELETE /v1/article/1/image/10` with a valid cookie -> 404, image 10 still exists. +5. `POST /v1/media/` with a valid PNG -> 201 with `id` and `url` (this stays covered by `media_library_http_test.go`; confirm the case still passes after the deletions). +6. `PATCH /v1/article/1/publish` still answers as in `verified_account_required_http_test.go`. + +## Cleanup (Go) +7. Delete `test/upload_article_image_http_test.go`. +8. In `test/verified_account_required_http_test.go`: remove the `api/handlers/image` import, the `imageHandler` construction (line 34), the route `r.Post("/{id}/image", ...)` (line 53) and the "unverified user is refused when attaching an image" case (around line 149). +9. Run `task test` and `task lint`; nothing else may reference `PostLegacyArticleImage` or `handlers/image`. + +## Manual verification +With `task dev`, `curl -i -X POST /v1/article/1/image` with a valid cookie answers 404. diff --git a/features/upload_article_image.feature b/features/upload_article_image.feature deleted file mode 100644 index 195930d..0000000 --- a/features/upload_article_image.feature +++ /dev/null @@ -1,47 +0,0 @@ -Feature: POST /v1/article/{id}/image is a thin alias that creates a library image - - As the current editor, until the editor slice moves it to /v1/media - I want to keep uploading through the article route and get {id, location} back - So that the editor keeps working while images are library assets - - Background: - Given the route stays in the article router's jwtauth group with RequireCurrentToken and - RequireVerifiedUser, and is marked for removal in the editor slice - And the handler calls GetArticleUseCase first, so an unknown article is 404 - apperr.ArticleNotFoundErrorMessage and nothing is created - And it then calls the same CreateImageUseCase as POST /v1/media/, with UploadedByID from the - jwt user_id claim, attaches nothing to the article, and answers 201 {"id", "location"} - where location is the image's url - And type, size and unreadable errors map as on /v1/media/: 415, 413, 400 - And DELETE /v1/article/{id}/image/{imageId} is not mounted - - Scenario: Uploading through the alias creates a library image - Given a valid jwt cookie for user 1 and article 1 "Backyard Renovation Update" - When the client sends POST /v1/article/1/image with a real PNG named "before.png" - Then the response status is 201 - And the body has "id" equal to the new image's id and "location" equal to its url - And GET /v1/media/ lists the image with uploaded_by of user 1 - And article 1 is unchanged - - Scenario: An unknown article is 404 and creates no image - When the client sends POST /v1/article/9999/image with a valid PNG - Then the response status is 404 and the body is "Article not found" - And the image repository's Create is never called - - Scenario Outline: Rejected files use the media status codes - When the client sends POST /v1/article/1/image with <file> - Then the response status is <status> and Create is never called - - Examples: - | file | status | - | "notes.pdf" as application/pdf | 415 | - | a valid PNG padded to 10485761 bytes | 413 | - | "broken.png" as image/png with text bytes | 400 | - - Scenario: The alias still requires authentication - When the client sends POST /v1/article/1/image with no cookie - Then the response status is 401 and Create is never called - - Scenario: The removed delete route is gone - When the client sends DELETE /v1/article/1/image/10 with a valid cookie - Then the response status is 405 and image 10 still exists diff --git a/features/upload_article_image.qa.md b/features/upload_article_image.qa.md deleted file mode 100644 index e7fae43..0000000 --- a/features/upload_article_image.qa.md +++ /dev/null @@ -1,21 +0,0 @@ -# QA procedure: upload_article_image - -Package: `test` (black-box, extending `article_test.go` / `image_test.go`). - -## Setup shared by every case -1. Call `validator.InitValidator()` once. -2. Wire `image.NewHandler(createImageUseCase, getArticleUseCase)` (no update or delete use cases) and mount only `POST /{id}/image` in the article group, behind `Verifier`, `Authenticator`, `RequireCurrentToken`, `RequireVerifiedUser`. -3. Use real image bytes generated in the test, as in `media_library.qa.md` step 8, and multipart bodies with an explicit part `Content-Type`. - -## HTTP checks -4. Valid PNG on article 1 -> 201 `{id, location}` matching the created image, and `articleRepo.articles[1]` has an unchanged body. -5. Unknown article 9999 -> 404 `"Article not found\n"`, `Create` never called. -6. Rejected files: `application/pdf` -> 415, PNG padded to 10485761 bytes -> 413, text bytes as `image/png` -> 400; `Create` never called. -7. No cookie -> 401, `Create` never called. -8. `DELETE /v1/article/1/image/10` on this router -> 405, image 10 still exists. - -## Cleanup -Delete `test/remove_article_image_http_test.go` and update `test/upload_article_image_http_test.go` to the cases above. - -## Manual verification -With a real instance, `POST /v1/article/{id}/image` writes a file to `UPLOAD_DIR` and a row visible in `GET /v1/media/`, and no article association is written. diff --git a/features/verified_account_required.feature b/features/verified_account_required.feature index 8f2c30d..6c8f487 100644 --- a/features/verified_account_required.feature +++ b/features/verified_account_required.feature @@ -12,12 +12,12 @@ Feature: Protected article routes and the offline-inclusive listing require a ve Given before this slice, the protected group inside NewArticleRouter (api/router.go) applies only jwtauth.Verifier then jwtauth.Authenticator to GET /v1/article/{id}, POST /v1/article/, DELETE /v1/article/{id}, PATCH /v1/article/{id}, PATCH /v1/article/{id}/publish, - PATCH /v1/article/{id}/unpublish and POST /v1/article/{id}/image, so any caller with a + PATCH /v1/article/{id}/unpublish, so any caller with a validly signed, unexpired jwt cookie reaches the handler regardless of that account's is_verified status And this slice adds a third middleware, api/middleware.RequireVerifiedUser(getUser *useCases.GetUserUseCase), mounted after jwtauth.Authenticator in that same group, so the - order becomes Verifier, Authenticator, RequireVerifiedUser for all seven routes above + order becomes Verifier, Authenticator, RequireVerifiedUser for all six routes above And RequireVerifiedUser reads user_id from the request's jwt claims using a new exported helper, api/pkg/claims.UserId(r *http.Request) (uint32, bool), which replaces the identical private userIdFromClaims method api/handlers/auth.Handler.Verify and @@ -81,16 +81,6 @@ Feature: Protected article routes and the offline-inclusive listing require a ve | PATCH | /v1/article/1/publish | | PATCH | /v1/article/1/unpublish | - Scenario: An unverified user is also refused when attaching an image - Given a user repository containing user id 1, is_verified false - And a valid jwt cookie for user id 1 - And the article repository contains an article with id 1 titled "Backyard Renovation Update" - When the client sends POST /v1/article/1/image with that cookie and a multipart form field - "file" holding a file named "before.png" with content type "image/png" - Then the response status is 403 - And the response body is the apperr.VerificationRequiredErrorMessage text - And the image repository's Create is never called - Scenario: A verified user still exercises the protected routes exactly as before Given a user repository containing user id 1, is_verified true And a valid jwt cookie for user id 1 diff --git a/features/verified_account_required.qa.md b/features/verified_account_required.qa.md index 423b3e8..1f135dc 100644 --- a/features/verified_account_required.qa.md +++ b/features/verified_account_required.qa.md @@ -29,8 +29,7 @@ routers, so the HTTP-level checks live in a new `test/verified_account_required_ - a nested `r.Group` adding `jwtauth.Authenticator(token)` then `middleware.RequireVerifiedUser(getUser)`, containing `r.Get("/{id}", ...)`, `r.Post("/", ...)`, `r.Delete("/{id}", ...)`, `r.Patch("/{id}", ...)`, - `r.Patch("/{id}/publish", ...)`, `r.Patch("/{id}/unpublish", ...)`, - `r.Post("/{id}/image", ...)` + `r.Patch("/{id}/publish", ...)`, `r.Patch("/{id}/unpublish", ...)` No live server. 8. Mint jwt cookies with `token.Encode(map[string]any{"user_id": uint32(1)})` for the valid-token cases, following the convention in `features/article_publishing.qa.md`. @@ -45,10 +44,6 @@ routers, so the HTTP-level checks live in a new `test/verified_account_required_ `PATCH /v1/article/1/unpublish`, each sent with the valid cookie from step 8 → assert status `403` and body exactly `apperr.VerificationRequiredErrorMessage`. Assert the article repository was not mutated by any of the write calls. -11. Same seed, `POST /v1/article/1/image` with the valid cookie and a multipart form field `"file"` - holding `before.png` / `image/png` → assert status `403`, body - `apperr.VerificationRequiredErrorMessage`, and the image repository's `Create` is never called - (mirroring the assertion style in `test/upload_article_image_test.go`). ## HTTP checks: a verified caller is unaffected @@ -127,7 +122,7 @@ routers, so the HTTP-level checks live in a new `test/verified_account_required_ ## Manual verification (not exercised by the Go test) - The SPA-visible behaviour of an unverified, logged-in user landing on the back-office article - screens (create/edit/publish/image-upload) and seeing a 403 instead of the normal editing UI, + screens (create/edit/publish) and seeing a 403 instead of the normal editing UI, and the front-office/back-office listing showing only published articles to that same unverified session, is out of scope for this slice's automated test and should be checked by hand against a running dev build with an unverified test account. diff --git a/test/remove_legacy_article_image_alias_http_test.go b/test/remove_legacy_article_image_alias_http_test.go new file mode 100644 index 0000000..b3e62b5 --- /dev/null +++ b/test/remove_legacy_article_image_alias_http_test.go @@ -0,0 +1,76 @@ +package test + +import ( + "net/http" + "testing" +) + +func TestRemoveLegacyArticleImageAliasHttp(t *testing.T) { + newEnv := func(t *testing.T) *mediaEnv { + env := newMediaEnv(t) + env.seedArticle(1, "Backyard Renovation Update", false, "unchanged body") + return env + } + + t.Run("The removed routes are not found", func(t *testing.T) { + examples := []struct { + name string + method string + path string + }{ + {"POST /v1/article/1/image", http.MethodPost, "/v1/article/1/image"}, + {"DELETE /v1/article/1/image/10", http.MethodDelete, "/v1/article/1/image/10"}, + } + for _, example := range examples { + t.Run(example.name, func(t *testing.T) { + env := newEnv(t) + + req := newMultipartFileRequest(t, example.method, example.path, "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", rec.Code) + } + if env.images.createCalls != 0 { + t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) + } + if len(env.images.images) != 0 { + t.Errorf("expected no image to exist, got %d", len(env.images.images)) + } + }) + } + }) + + t.Run("Uploading still works through the media route", func(t *testing.T) { + env := newEnv(t) + + req := newMultipartFileRequest(t, http.MethodPost, "/v1/media/", "before.png", "image/png", pngBytes(t, 3, 2)) + rec := env.serve(t, req, mediaVerifiedUserId) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) + } + got := decodeObject(t, rec.Body.String()) + if got["url"] == nil || numberOf(got["id"]) != 1 { + t.Errorf("expected a library item with a url and id 1, got %v", got) + } + listing := env.send(t, http.MethodGet, "/v1/media/", "", mediaVerifiedUserId) + items := decodeObject(t, listing.Body.String())["items"].([]any) + if len(items) != 1 { + t.Errorf("expected the list to hold the upload, got %v", items) + } + }) + + t.Run("The rest of the article router is unchanged", func(t *testing.T) { + env := newEnv(t) + + rec := env.send(t, http.MethodPatch, "/v1/article/1/publish", "", mediaVerifiedUserId) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String()) + } + if !env.articles.articles[1].IsOnline { + t.Errorf("expected article 1 to be online") + } + }) +} diff --git a/test/upload_article_image_http_test.go b/test/upload_article_image_http_test.go deleted file mode 100644 index 6d977b1..0000000 --- a/test/upload_article_image_http_test.go +++ /dev/null @@ -1,176 +0,0 @@ -package test - -import ( - "RenewCMS/api/pkg/apperr" - "net/http" - "testing" - - domainImage "RenewCMS/internal/domain/image" -) - -func TestUploadArticleImageHttp(t *testing.T) { - newEnv := func(t *testing.T) *mediaEnv { - env := newMediaEnv(t) - env.seedArticle(1, "Backyard Renovation Update", true, "unchanged body") - return env - } - - t.Run("Uploading through the alias creates a library image", func(t *testing.T) { - env := newEnv(t) - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) - rec := env.serve(t, req, mediaVerifiedUserId) - - if rec.Code != http.StatusCreated { - t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) - } - got := decodeObject(t, rec.Body.String()) - if len(env.images.images) != 1 { - t.Fatalf("expected exactly one library image, got %d", len(env.images.images)) - } - created := env.images.images[1] - if numberOf(got["id"]) != created.ID { - t.Errorf("expected id %d, got %v", created.ID, got["id"]) - } - if got["location"] != created.Path { - t.Errorf("expected location %q, got %v", created.Path, got["location"]) - } - if created.UploadedByID != mediaVerifiedUserId { - t.Errorf("expected UploadedByID 1, got %d", created.UploadedByID) - } - - listing := env.send(t, http.MethodGet, "/v1/media/", "", mediaVerifiedUserId) - items := decodeObject(t, listing.Body.String())["items"].([]any) - if len(items) != 1 || items[0].(map[string]any)["uploaded_by"] != "Inès Duarte" { - t.Errorf("expected the media list to hold the image uploaded by user 1, got %v", items) - } - if env.articles.articles[1].Body != "unchanged body" { - t.Errorf("expected article 1 to be unchanged, got %+v", env.articles.articles[1]) - } - }) - - t.Run("An unknown article is 404 and creates no image", func(t *testing.T) { - env := newEnv(t) - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/9999/image", "before.png", "image/png", pngBytes(t, 3, 2)) - rec := env.serve(t, req, mediaVerifiedUserId) - - if rec.Code != http.StatusNotFound { - t.Fatalf("expected status 404, got %d", rec.Code) - } - if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if env.images.createCalls != 0 { - t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) - } - }) - - t.Run("Rejected files use the media status codes", func(t *testing.T) { - examples := []struct { - name string - filename string - contentType string - content []byte - status int - }{ - {"notes.pdf as application/pdf", "notes.pdf", "application/pdf", []byte("%PDF"), http.StatusUnsupportedMediaType}, - {"a valid PNG padded to 10485761 bytes", "huge.png", "image/png", paddedPngBytes(t, int(domainImage.MaxImageSize)+1), http.StatusRequestEntityTooLarge}, - {"broken.png as image/png with text bytes", "broken.png", "image/png", []byte("not an image"), http.StatusBadRequest}, - } - for _, example := range examples { - t.Run(example.name, func(t *testing.T) { - env := newEnv(t) - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", example.filename, example.contentType, example.content) - rec := env.serve(t, req, mediaVerifiedUserId) - - if rec.Code != example.status { - t.Fatalf("expected status %d, got %d", example.status, rec.Code) - } - if env.images.createCalls != 0 { - t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) - } - }) - } - }) - - t.Run("The alias still requires authentication", func(t *testing.T) { - env := newEnv(t) - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) - rec := env.serve(t, req, 0) - - if rec.Code != http.StatusUnauthorized { - t.Fatalf("expected status 401, got %d", rec.Code) - } - if env.images.createCalls != 0 { - t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) - } - }) - - t.Run("The removed delete route is gone", func(t *testing.T) { - env := newEnv(t) - env.seedImage(10, "before.png", 10, "image/png", 3, 2, sha256Hex([]byte("x")), 1) - - rec := env.send(t, http.MethodDelete, "/v1/article/1/image/10", "", mediaVerifiedUserId) - - if rec.Code != http.StatusNotFound && rec.Code != http.StatusMethodNotAllowed { - t.Fatalf("expected status 404 or 405, got %d", rec.Code) - } - if _, ok := env.images.images[10]; !ok { - t.Errorf("expected image 10 to still exist") - } - if len(env.images.deleteCalls) != 0 { - t.Errorf("expected Delete to never be called, got %v", env.images.deleteCalls) - } - }) - - t.Run("Uploading with a non-numeric article id answers 400", func(t *testing.T) { - env := newEnv(t) - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/abc/image", "before.png", "image/png", pngBytes(t, 3, 2)) - rec := env.serve(t, req, mediaVerifiedUserId) - - if rec.Code != http.StatusBadRequest || rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { - t.Fatalf("expected 400 with the id message, got %d %q", rec.Code, rec.Body.String()) - } - }) - - t.Run("Uploading without a file field answers 400", func(t *testing.T) { - env := newEnv(t) - - rec := env.serve(t, newMultipartWithoutFileRequest(t, "/v1/article/1/image"), mediaVerifiedUserId) - - if rec.Code != http.StatusBadRequest { - t.Fatalf("expected status 400, got %d", rec.Code) - } - if env.images.createCalls != 0 { - t.Errorf("expected Create to never be called, got %d calls", env.images.createCalls) - } - }) - - t.Run("A repository failure while fetching the article answers 500", func(t *testing.T) { - env := newEnv(t) - env.articles.fail.failNext("Get") - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) - rec := env.serve(t, req, mediaVerifiedUserId) - - if rec.Code != http.StatusInternalServerError { - t.Fatalf("expected status 500, got %d", rec.Code) - } - }) - - t.Run("A repository failure while creating the image answers 500", func(t *testing.T) { - env := newEnv(t) - env.images.fail.failNext("Create") - - req := newMultipartFileRequest(t, http.MethodPost, "/v1/article/1/image", "before.png", "image/png", pngBytes(t, 3, 2)) - rec := env.serve(t, req, mediaVerifiedUserId) - - if rec.Code != http.StatusInternalServerError { - t.Fatalf("expected status 500, got %d", rec.Code) - } - }) -} diff --git a/test/verified_account_required_http_test.go b/test/verified_account_required_http_test.go index 761918f..fa7a437 100644 --- a/test/verified_account_required_http_test.go +++ b/test/verified_account_required_http_test.go @@ -2,7 +2,6 @@ package test import ( "RenewCMS/api/handlers/article" - "RenewCMS/api/handlers/image" "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" @@ -11,8 +10,6 @@ import ( "RenewCMS/internal/infrastructure/useCases" "bytes" "encoding/json" - "io" - "mime/multipart" "net/http" "net/http/httptest" "testing" @@ -21,7 +18,7 @@ import ( "github.com/go-chi/jwtauth/v5" ) -func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { +func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux { getUserUseCase := useCases.NewGetUserUseCase(userRepo) articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), @@ -31,10 +28,6 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo useCases.NewUpdateArticleUseCase(articleRepo), getUserUseCase, ) - imageHandler := image.NewHandler( - useCases.NewCreateImageUseCase(imageRepo), - useCases.NewGetArticleUseCase(articleRepo), - ) r := chi.NewRouter() r.Route("/v1", func(r chi.Router) { @@ -50,7 +43,6 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo r.Patch("/{id}", articleHandler.UpdateArticleBody) r.Patch("/{id}/publish", articleHandler.PublishArticle) r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle) - r.Post("/{id}/image", imageHandler.PostLegacyArticleImage) }) }) }) @@ -106,7 +98,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { t.Run("An authenticated but unverified user is refused on every protected article route: "+tc.method+" "+tc.path, func(t *testing.T) { articleRepo := seedVerifiedAccountRequiredArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(false) - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(tc.method, tc.path, tc.body) req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) @@ -125,51 +117,10 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { }) } - t.Run("An unverified user is also refused when attaching an image", func(t *testing.T) { - articleRepo := seedVerifiedAccountRequiredArticleRepo() - userRepo := seedVerifiedAccountRequiredUserRepo(false) - imageRepo := newMockImageRepo() - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, imageRepo, token) - - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - part, err := writer.CreatePart(map[string][]string{ - "Content-Disposition": {`form-data; name="file"; filename="before.png"`}, - "Content-Type": {"image/png"}, - }) - if err != nil { - t.Fatalf("failed to create multipart part: %v", err) - } - if _, err := io.WriteString(part, "fake-image-data"); err != nil { - t.Fatalf("failed to write file content: %v", err) - } - if err := writer.Close(); err != nil { - t.Fatalf("failed to close multipart writer: %v", err) - } - req := httptest.NewRequest(http.MethodPost, "/v1/article/1/image", body) - req.Header.Set("Content-Type", writer.FormDataContentType()) - req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) - rec := httptest.NewRecorder() - router.ServeHTTP(rec, req) - - if rec.Code != http.StatusForbidden { - t.Fatalf("expected status 403, got %d, body %q", rec.Code, rec.Body.String()) - } - if rec.Body.String() != apperr.VerificationRequiredErrorMessage+"\n" { - t.Errorf("unexpected body: %q", rec.Body.String()) - } - if imageRepo.images == nil { - t.Fatalf("expected imageRepo.images to be initialized") - } - if len(imageRepo.images) != 0 { - t.Errorf("expected the image repository's Create to never be called, got %d images", len(imageRepo.images)) - } - }) - t.Run("A verified user still exercises the protected routes exactly as before", func(t *testing.T) { articleRepo := newMockArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(true) - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`) req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) @@ -202,7 +153,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { t.Run("A token that resolves to no usable account still answers 401, never 403: "+tc.name, func(t *testing.T) { articleRepo := seedVerifiedAccountRequiredArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(false) - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`) req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, tc.claims)) @@ -222,7 +173,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { articleRepo := newMockArticleRepo() userRepo := newMockUserRepo() userRepo.users[7] = domainUser.User{ID: 7, IsVerified: true} - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`) req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": 7.5})) @@ -250,7 +201,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { t.Run("A request with no jwt cookie is still refused with 401, never 403: "+tc.method+" "+tc.path, func(t *testing.T) { articleRepo := seedVerifiedAccountRequiredArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(false) - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(tc.method, tc.path, "") rec := httptest.NewRecorder() @@ -266,7 +217,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { articleRepo := seedVerifiedAccountRequiredArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(false) userRepo.fail.failNext("Get") - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodDelete, "/v1/article/1", "") req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) @@ -308,7 +259,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { t.Run("Listing falls back to the online-only list for an authenticated but unverified caller", func(t *testing.T) { articleRepo := seedListingArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(false) - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) @@ -321,7 +272,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { t.Run("Listing still returns every article for an authenticated and verified caller", func(t *testing.T) { articleRepo := seedListingArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(true) - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) @@ -347,7 +298,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { t.Run("Listing falls back to the online-only list when the caller's account no longer exists", func(t *testing.T) { articleRepo := seedListingArticleRepo() userRepo := newMockUserRepo() - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(999)})) @@ -361,7 +312,7 @@ func TestVerifiedAccountRequiredHttp(t *testing.T) { articleRepo := seedListingArticleRepo() userRepo := seedVerifiedAccountRequiredUserRepo(true) userRepo.fail.failNext("Get") - router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, token) req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) diff --git a/web/src/components/ArticleDetails.vue b/web/src/components/ArticleDetails.vue index 0e7b707..09f5557 100644 --- a/web/src/components/ArticleDetails.vue +++ b/web/src/components/ArticleDetails.vue @@ -2,6 +2,7 @@ import { computed } from 'vue' import { useToast } from '@nuxt/ui/composables' import type { Article } from '@/composables/useArticles' +import { bodyImageCount } from '@/lib/bodyImageIds' const props = defineProps<{ article: Article }>() @@ -50,7 +51,7 @@ function formatDetailDate(iso: string, relative = false) { const createdLabel = computed(() => formatDetailDate(props.article.created_at)) const updatedLabel = computed(() => formatDetailDate(props.article.updated_at, true)) const publicUrl = computed(() => `/v1/article/${props.article.id}`) -const imagesCount = computed(() => props.article.images.length) +const imagesCount = computed(() => bodyImageCount(props.article.body)) const apiNote = computed(() => props.article.is_online ? 'Live. Your website can read this article.' diff --git a/web/src/components/ArticleEditor.vue b/web/src/components/ArticleEditor.vue index 22e9a88..474ff3e 100644 --- a/web/src/components/ArticleEditor.vue +++ b/web/src/components/ArticleEditor.vue @@ -208,6 +208,18 @@ defineExpose({ insertImage(src: string, alt: string, imageId: number) { editorInstance.value?.commands.setArticleImage({ src, alt, imageId }) }, + imageSrc(imageId: number): string | null { + const editor = editorInstance.value + if (!editor) { + return null + } + const pos = findArticleImagePos(editor, imageId) + if (pos === null) { + return null + } + const src = editor.state.doc.nodeAt(pos)?.attrs.src + return typeof src === 'string' ? src : null + }, removeImage(imageId: number) { const editor = editorInstance.value if (!editor) { diff --git a/web/src/components/ArticleEditorImageNode.vue b/web/src/components/ArticleEditorImageNode.vue index 6128743..3e38f76 100644 --- a/web/src/components/ArticleEditorImageNode.vue +++ b/web/src/components/ArticleEditorImageNode.vue @@ -41,13 +41,14 @@ function onDeleteClick() { v-if="showDeleteButton" data-testid="remove-image-button" icon="i-lucide-trash-2" - color="error" - variant="ghost" - size="xs" - class="absolute top-1 right-1 bg-default" - aria-label="Remove image" + color="neutral" + variant="outline" + class="absolute top-2 right-2 h-7 gap-1.5 border border-accented bg-default px-2.5 text-xs font-medium text-highlighted ring-0" + :ui="{ leadingIcon: 'size-3.5' }" @mousedown.prevent @click="onDeleteClick" - /> + > + Remove image + </UButton> </NodeViewWrapper> </template> diff --git a/web/src/components/ConfirmDialog.vue b/web/src/components/ConfirmDialog.vue index ed6f5a9..aad9bd7 100644 --- a/web/src/components/ConfirmDialog.vue +++ b/web/src/components/ConfirmDialog.vue @@ -1,5 +1,6 @@ <script setup lang="ts"> import { computed, nextTick, onMounted, ref, watch } from 'vue' +import MediaThumb from '@/components/MediaThumb.vue' const props = withDefaults( defineProps<{ @@ -11,6 +12,7 @@ const props = withDefaults( tone: 'danger' | 'neutral' action: () => Promise<void | boolean> dismissResolvesTo?: 'cancel' | 'confirm' + preview?: { src: string; filename?: string; size?: string } }>(), { dismissResolvesTo: 'cancel', @@ -25,6 +27,14 @@ const error = ref('') const cancelButton = ref<{ $el: HTMLElement } | null>(null) let settled = false +const previewDetail = computed(() => + props.preview?.size ? `Stays in the library · ${props.preview.size}` : 'Stays in the library', +) +const previewThumb = computed(() => ({ + url: props.preview?.src ?? '', + content_type: '', + sha256: '', +})) const confirmColor = computed(() => (props.tone === 'danger' ? 'error' : 'primary')) const confirmText = computed(() => (pending.value ? props.pendingLabel : props.confirmLabel)) @@ -107,6 +117,25 @@ function onCancel() { role="alert" :ui="{ root: 'gap-2 px-3 py-2.5', description: 'text-sm text-toned' }" /> + <div + v-if="preview" + data-testid="confirm-preview" + class="flex items-center gap-3 border border-default px-3 py-2.5" + > + <MediaThumb + :item="previewThumb" + size="sm" + class="h-10.5 w-14 border border-default" + /> + <div class="flex min-w-0 flex-col"> + <span + v-if="preview.filename" + class="truncate font-medium text-highlighted" + >{{ preview.filename }}</span + > + <span class="text-xs/5 text-muted">{{ previewDetail }}</span> + </div> + </div> <slot name="body" :pending="pending" :confirm="onConfirm" /> </div> </template> diff --git a/web/src/components/ImageUploader.vue b/web/src/components/ImageUploader.vue index 6fa5e68..5616d5f 100644 --- a/web/src/components/ImageUploader.vue +++ b/web/src/components/ImageUploader.vue @@ -1,13 +1,21 @@ <script setup lang="ts"> import { computed, nextTick, onUnmounted, ref, watch } from 'vue' +import MediaPicker from '@/components/MediaPicker.vue' +import MediaThumb from '@/components/MediaThumb.vue' +import { ApiError, useApi } from '@/composables/useApi' +import { useLibraryList } from '@/composables/useLibraryList' +import { classifyMediaError, type MediaItem } from '@/composables/useMedia' +import { oldestOf } from '@/composables/useMediaUpload' +import { hashFile } from '@/lib/fileHash' import { IMAGE_TYPE_REJECTION_TEXT, formatUploadProgress, - isSizeRejectionBody, rejectionTitle, sizeRejectionText, validateImageFile, } from '@/lib/imageUpload' +import { libraryMatchText } from '@/lib/mediaFormat' +import { UNDERLINE_TAB_TRIGGER } from '@/lib/tabStyles' import { ASPECT_RATIOS, WIDTH_PRESETS, @@ -35,17 +43,38 @@ import { type Corner = 'tl' | 'tr' | 'bl' | 'br' import { useImageUpload } from '@/composables/useImageUpload' -const props = defineProps<{ - articleId: number - initialFile?: File | null -}>() +const props = withDefaults( + defineProps<{ + initialFile?: File | null + inArticleIds?: number[] + }>(), + { initialFile: null, inArticleIds: () => [] }, +) const emit = defineEmits<{ - uploaded: [location: string, imageId: number] + uploaded: [item: MediaItem] + inserted: [item: MediaItem, alt: string] close: [] }>() -type Phase = 'idle' | 'dragging-over' | 'crop' | 'uploading' | 'rejected' | 'failed' +type Phase = 'idle' | 'dragging-over' | 'duplicate' | 'crop' | 'uploading' | 'rejected' | 'failed' +type Tab = 'upload' | 'library' + +const DROP_ZONE_UI = { + base: 'min-h-45 rounded-none border-accented p-4 text-sm', + wrapper: 'gap-2 p-0', + avatar: 'size-6 bg-transparent', + icon: 'size-6 text-toned', + label: 'mt-0 font-medium text-highlighted', + description: 'mt-0 max-w-75 text-xs/5 text-toned', +} + +const FOOTER_BUTTON_CLASS = 'h-11 justify-center text-sm sm:h-8 sm:w-auto sm:px-3' + +const TAB_ITEMS = [ + { label: 'Upload', value: 'upload', icon: 'i-lucide-upload' }, + { label: 'Library', value: 'library', icon: 'i-lucide-images' }, +] const open = defineModel<boolean>('open', { default: false }) @@ -55,6 +84,11 @@ const lastUploadedFile = ref<File | null>(null) const pickedFile = ref<File | null>(null) const rejectionKind = ref<'type' | 'size' | null>(null) const succeeded = ref(false) +const tab = ref<Tab>('library') +const existing = ref<MediaItem | null>(null) +const selected = ref<MediaItem | null>(null) +const alt = ref('') +let tabTouched = false const naturalWidth = ref(0) const naturalHeight = ref(0) @@ -67,6 +101,8 @@ const previewCanvasRef = ref<HTMLCanvasElement | null>(null) const sourceBitmap = ref<ImageBitmap | null>(null) const { progress, start, abort } = useImageUpload() +const { request } = useApi() +const library = useLibraryList() let closeTimer: ReturnType<typeof setTimeout> | null = null const uploadPercent = computed(() => { @@ -90,15 +126,43 @@ const rejectedText = computed(() => { return IMAGE_TYPE_REJECTION_TEXT }) +const onLibraryTab = computed(() => tab.value === 'library') + +const showTabs = computed( + () => + onLibraryTab.value || + phase.value === 'idle' || + phase.value === 'dragging-over' || + phase.value === 'duplicate', +) + +const tabModel = computed({ + get: () => tab.value, + set: (value: string) => selectTab(value === 'library' ? 'library' : 'upload'), +}) + +const duplicateText = computed(() => + existing.value && chosenFile.value + ? libraryMatchText(chosenFile.value.name, existing.value) + : '', +) + const cancelLabel = computed(() => (phase.value === 'uploading' ? 'Cancel upload' : 'Cancel')) const showCancel = computed( () => - phase.value !== 'failed' && - phase.value !== 'crop' && - !(phase.value === 'uploading' && succeeded.value), + onLibraryTab.value || + (phase.value !== 'failed' && + phase.value !== 'crop' && + !(phase.value === 'uploading' && succeeded.value)), ) +const showFailedActions = computed(() => !onLibraryTab.value && phase.value === 'failed') +const showRejectedActions = computed(() => !onLibraryTab.value && phase.value === 'rejected') +const showCropActions = computed(() => !onLibraryTab.value && phase.value === 'crop') + +const wideModal = computed(() => showTabs.value || phase.value === 'crop') + const modalTitle = computed(() => (phase.value === 'crop' ? 'Crop and resize' : 'Add an image')) const modalDescription = computed(() => { @@ -110,11 +174,13 @@ const modalDescription = computed(() => { const modalContentClass = computed( () => - `divide-y-0 shadow-(--shadow-modal) ring-0 sm:top-35 sm:translate-y-0 max-sm:!inset-x-4 max-sm:!top-auto max-sm:!bottom-4 max-sm:!left-4 max-sm:!w-auto max-sm:!max-w-none max-sm:!translate-x-0 max-sm:!translate-y-0 ${ - phase.value === 'crop' ? 'sm:max-w-160' : 'sm:max-w-120' - }`, + `divide-y-0 shadow-(--shadow-modal) ring-0 sm:translate-y-0 max-sm:!inset-x-4 max-sm:!top-auto max-sm:!bottom-4 max-sm:!left-4 max-sm:!w-auto max-sm:!max-w-none max-sm:!translate-x-0 max-sm:!translate-y-0 ${ + showTabs.value ? 'sm:top-10' : 'sm:top-35' + } ${wideModal.value ? 'sm:max-w-160' : 'sm:max-w-120'}`, ) +const bodyClass = computed(() => (showTabs.value ? 'px-6 pt-4' : 'px-6 pt-5')) + const boundsWidth = computed( () => rotatedDimensions(naturalWidth.value, naturalHeight.value, rotation.value).width, ) @@ -164,20 +230,24 @@ function clearCloseTimer() { async function runUpload(file: File) { lastUploadedFile.value = file - const outcome = await start(props.articleId, file) + const outcome = await start(file) if (outcome.kind === 'success') { succeeded.value = true - emit('uploaded', outcome.location, outcome.id) + emit('uploaded', outcome.item) closeTimer = setTimeout(() => { open.value = false }, 600) return } if (outcome.kind === 'rejected') { - rejectionKind.value = isSizeRejectionBody(outcome.body) ? 'size' : 'type' + rejectionKind.value = outcome.reason phase.value = 'rejected' return } + if (outcome.kind === 'handled') { + open.value = false + return + } phase.value = 'failed' } @@ -244,7 +314,32 @@ async function openCropStep(file: File) { await loadPreviewBitmap(file) } -function handleFile(file: File) { +async function findExisting(file: File): Promise<MediaItem | null | 'handled'> { + const hash = await hashFile(file) + if (hash === null) { + return null + } + try { + const response = await request<{ items: MediaItem[] }>(`/v1/media/?hash=${hash}`) + return oldestOf(response.items) ?? null + } catch (error) { + if (error instanceof ApiError && classifyMediaError(error) === 'handled') { + return 'handled' + } + return null + } +} + +function continueWithFile(file: File) { + if (file.type === 'image/svg+xml') { + phase.value = 'uploading' + runUpload(file) + return + } + openCropStep(file) +} + +async function handleFile(file: File) { chosenFile.value = file const result = validateImageFile(file) if (!result.ok) { @@ -254,12 +349,75 @@ function handleFile(file: File) { } rejectionKind.value = null succeeded.value = false - if (file.type === 'image/svg+xml') { - phase.value = 'uploading' - runUpload(file) + const match = await findExisting(file) + if (!open.value || chosenFile.value !== file) { return } - openCropStep(file) + if (match === 'handled') { + open.value = false + return + } + tab.value = 'upload' + if (match) { + existing.value = match + phase.value = 'duplicate' + return + } + continueWithFile(file) +} + +function uploadAnyway() { + if (chosenFile.value) { + existing.value = null + continueWithFile(chosenFile.value) + } +} + +function selectTab(next: Tab) { + tabTouched = true + tab.value = next + if (next === 'library') { + library.ensureLoaded() + } +} + +function useExisting() { + if (!existing.value) { + return + } + selectItem(existing.value) + selectTab('library') + phase.value = 'idle' + existing.value = null +} + +function selectItem(item: MediaItem) { + if (selected.value?.id === item.id) { + return + } + selected.value = item + alt.value = '' +} + +async function searchLibrary(query: string) { + selected.value = null + alt.value = '' + await library.search(query) +} + +function insertSelected() { + if (!selected.value) { + return + } + emit('inserted', selected.value, alt.value) + open.value = false +} + +async function openLibraryFirst() { + await library.load() + if (!tabTouched && library.status.value === 'ready' && library.items.value.length === 0) { + tab.value = 'upload' + } } function onFileChange() { @@ -291,6 +449,7 @@ function chooseAnotherFile() { chosenFile.value = null pickedFile.value = null rejectionKind.value = null + existing.value = null closeSourceBitmap() } @@ -517,9 +676,18 @@ watch(open, (isOpen) => { pickedFile.value = null rejectionKind.value = null succeeded.value = false + existing.value = null + selected.value = null + alt.value = '' + tabTouched = false + library.reset() if (props.initialFile) { + tab.value = 'upload' handleFile(props.initialFile) + return } + tab.value = 'library' + openLibraryFirst() return } if (phase.value === 'uploading' && !succeeded.value) { @@ -542,262 +710,375 @@ onUnmounted(() => { :description="modalDescription" :ui="{ content: modalContentClass, + header: 'min-h-12 items-start px-6 pt-5 pb-0 sm:px-6', + close: 'top-5 end-6', title: 'text-lg/6 font-semibold', - body: 'px-6 pt-5 pb-0 sm:px-6 sm:pt-5 sm:pb-0', + body: 'p-0 sm:p-0', footer: 'px-6 py-5 sm:px-6', }" > <template #body> - <div - v-if="phase === 'idle' || phase === 'dragging-over'" - data-testid="image-dropzone" - @dragover.prevent="onDragOver" - @dragleave.prevent="onDragLeave" - @drop.prevent="onDropOnZone" - > - <UFileUpload - v-model="pickedFile" - data-testid="image-file-input" - accept="image/png,image/jpeg,image/webp,image/svg+xml" - :multiple="false" - layout="list" - :reset="true" - @change="onFileChange" - > - <template #label> - {{ phase === 'dragging-over' ? 'Drop to upload' : 'Drop an image here' }} - </template> - <template #description>PNG, JPEG, WEBP, SVG · max 10 MB</template> - <template #actions="{ open: openDialog }"> - <UButton color="neutral" variant="outline" @click="openDialog()"> - Choose file - </UButton> - </template> - </UFileUpload> + <div v-if="showTabs" class="mt-4 border-b border-default px-6"> + <UTabs + v-model="tabModel" + :items="TAB_ITEMS" + variant="link" + color="neutral" + :content="false" + class="-mb-px" + :ui="{ + list: 'gap-5 border-b-0 p-0', + trigger: `${UNDERLINE_TAB_TRIGGER} max-md:text-sm`, + leadingIcon: 'size-3.5', + }" + /> </div> + <div :class="bodyClass"> + <MediaPicker + v-if="onLibraryTab" + v-model:alt="alt" + :items="library.items.value" + :status="library.status.value" + :show-skeleton="library.showSkeleton.value" + :loading-more="library.loadingMore.value" + :has-more="library.nextCursor.value !== null" + :total="library.total.value" + :query="library.query.value" + :first-new-id="library.firstNewId.value" + :in-article-ids="inArticleIds" + :selected="selected" + @search="searchLibrary" + @select="selectItem" + @load-more="library.loadMore" + @retry="library.load()" + @upload="selectTab('upload')" + /> - <div v-else-if="phase === 'crop'" class="flex flex-col gap-4"> <div - class="flex h-60 items-center justify-center bg-elevated p-4 sm:h-85" - data-testid="crop-stage" + v-else-if="phase === 'idle' || phase === 'dragging-over'" + data-testid="image-dropzone" + @dragover.prevent="onDragOver" + @dragleave.prevent="onDragLeave" + @drop.prevent="onDropOnZone" > - <div - ref="imageBoxRef" - class="relative h-full max-w-full" - :style="{ aspectRatio: `${boundsWidth} / ${boundsHeight}` }" - :aria-label="`${chosenFile?.name}, ${boundsWidth} × ${boundsHeight} px`" - role="img" + <UFileUpload + v-model="pickedFile" + data-testid="image-file-input" + accept="image/png,image/jpeg,image/webp,image/svg+xml" + :multiple="false" + layout="list" + icon="i-lucide-image-plus" + :reset="true" + :ui="DROP_ZONE_UI" + @change="onFileChange" > - <div class="absolute inset-0 overflow-hidden bg-(--color-sand-200)"> - <canvas ref="previewCanvasRef" class="absolute inset-0 h-full w-full" /> + <template #label> + {{ + phase === 'dragging-over' + ? 'Drop to upload' + : 'Drop an image here, or choose a file' + }} + </template> + <template #description>PNG, JPEG, WEBP, SVG · max 10 MB</template> + </UFileUpload> + </div> + + <div + v-else-if="phase === 'duplicate' && existing" + data-testid="duplicate-card" + class="flex flex-col gap-3 border border-info bg-default p-4" + > + <div class="flex items-center gap-3"> + <MediaThumb + :item="existing" + size="sm" + class="h-13.5 w-18 border border-default" + /> + <div class="flex min-w-0 flex-col gap-0.5"> + <p class="font-semibold text-highlighted">Already in your library</p> + <p class="text-xs text-toned">{{ duplicateText }}</p> + </div> + </div> + <div class="flex flex-col gap-2 md:flex-row"> + <UButton class="h-11 justify-center md:h-8" @click="useExisting"> + Use existing + </UButton> + <UButton + color="neutral" + variant="outline" + class="h-11 justify-center border border-accented bg-default text-highlighted ring-0 md:h-8" + @click="uploadAnyway" + > + Upload anyway + </UButton> + </div> + </div> + + <div v-else-if="phase === 'crop'" class="flex flex-col gap-4"> + <div + class="flex h-60 items-center justify-center bg-elevated p-4 sm:h-85" + data-testid="crop-stage" + > + <div + ref="imageBoxRef" + class="relative h-full max-w-full" + :style="{ aspectRatio: `${boundsWidth} / ${boundsHeight}` }" + :aria-label="`${chosenFile?.name}, ${boundsWidth} × ${boundsHeight} px`" + role="img" + > + <div class="absolute inset-0 overflow-hidden bg-(--color-sand-200)"> + <canvas + ref="previewCanvasRef" + class="absolute inset-0 h-full w-full" + /> + <div + class="absolute" + :style="{ + left: `${(rect.x / boundsWidth) * 100}%`, + top: `${(rect.y / boundsHeight) * 100}%`, + width: `${(rect.width / boundsWidth) * 100}%`, + height: `${(rect.height / boundsHeight) * 100}%`, + boxShadow: '0 0 0 2000px rgb(10 10 10 / 0.55)', + }" + /> + </div> <div - class="absolute" + data-testid="crop-frame" + tabindex="0" + role="group" + aria-label="Crop area. Arrow keys move it, Shift and arrow keys resize it." + class="absolute cursor-move touch-none outline outline-white" :style="{ left: `${(rect.x / boundsWidth) * 100}%`, top: `${(rect.y / boundsHeight) * 100}%`, width: `${(rect.width / boundsWidth) * 100}%`, height: `${(rect.height / boundsHeight) * 100}%`, - boxShadow: '0 0 0 2000px rgb(10 10 10 / 0.55)', }" - /> + @keydown="onFrameKeydown" + @mousedown="onFrameDragStart" + @touchstart="onFrameDragStart" + > + <div + class="pointer-events-none absolute top-0 bottom-0 w-px bg-white/45" + style="left: 33.333%" + /> + <div + class="pointer-events-none absolute top-0 bottom-0 w-px bg-white/45" + style="left: 66.666%" + /> + <div + class="pointer-events-none absolute left-0 right-0 h-px bg-white/45" + style="top: 33.333%" + /> + <div + class="pointer-events-none absolute left-0 right-0 h-px bg-white/45" + style="top: 66.666%" + /> + <div + v-for="corner in ['tl', 'tr', 'bl', 'br'] as Corner[]" + :key="corner" + data-testid="crop-handle" + class="absolute size-3 touch-none border border-ink-900 bg-white" + :class="{ + '-top-1.5 -left-1.5 cursor-nwse-resize': corner === 'tl', + '-top-1.5 -right-1.5 cursor-nesw-resize': corner === 'tr', + '-bottom-1.5 -left-1.5 cursor-nesw-resize': corner === 'bl', + '-bottom-1.5 -right-1.5 cursor-nwse-resize': + corner === 'br', + }" + @mousedown="onHandleDragStart($event, corner)" + @touchstart="onHandleDragStart($event, corner)" + /> + <span + class="absolute bottom-2 left-2 bg-ink-900/80 px-1.5 py-px font-mono text-xs whitespace-nowrap text-white" + > + {{ dimsLabel }} + </span> + </div> + </div> + </div> + + <div class="flex flex-col gap-1.5"> + <div class="flex items-center gap-1"> + <span class="flex-1 font-medium text-highlighted">Aspect ratio</span> + <UButton + color="neutral" + variant="ghost" + size="sm" + icon="i-lucide-rotate-ccw-square" + @click="rotateImage" + > + Rotate + </UButton> + <UButton + color="neutral" + variant="ghost" + size="sm" + icon="i-lucide-undo-2" + @click="resetCrop" + > + Reset + </UButton> </div> <div - data-testid="crop-frame" - tabindex="0" - role="group" - aria-label="Crop area. Arrow keys move it, Shift and arrow keys resize it." - class="absolute cursor-move touch-none outline outline-white" - :style="{ - left: `${(rect.x / boundsWidth) * 100}%`, - top: `${(rect.y / boundsHeight) * 100}%`, - width: `${(rect.width / boundsWidth) * 100}%`, - height: `${(rect.height / boundsHeight) * 100}%`, - }" - @keydown="onFrameKeydown" - @mousedown="onFrameDragStart" - @touchstart="onFrameDragStart" + role="radiogroup" + aria-label="Aspect ratio" + class="flex w-full border border-accented sm:w-max" > - <div - class="pointer-events-none absolute top-0 bottom-0 w-px bg-white/45" - style="left: 33.333%" - /> - <div - class="pointer-events-none absolute top-0 bottom-0 w-px bg-white/45" - style="left: 66.666%" - /> - <div - class="pointer-events-none absolute left-0 right-0 h-px bg-white/45" - style="top: 33.333%" - /> - <div - class="pointer-events-none absolute left-0 right-0 h-px bg-white/45" - style="top: 66.666%" - /> - <div - v-for="corner in ['tl', 'tr', 'bl', 'br'] as Corner[]" - :key="corner" - data-testid="crop-handle" - class="absolute size-3 touch-none border border-ink-900 bg-white" - :class="{ - '-top-1.5 -left-1.5 cursor-nwse-resize': corner === 'tl', - '-top-1.5 -right-1.5 cursor-nesw-resize': corner === 'tr', - '-bottom-1.5 -left-1.5 cursor-nesw-resize': corner === 'bl', - '-bottom-1.5 -right-1.5 cursor-nwse-resize': corner === 'br', - }" - @mousedown="onHandleDragStart($event, corner)" - @touchstart="onHandleDragStart($event, corner)" - /> - <span - class="absolute bottom-2 left-2 bg-ink-900/80 px-1.5 py-px font-mono text-xs whitespace-nowrap text-white" + <button + v-for="ratio in ASPECT_RATIOS" + :key="ratio" + type="button" + role="radio" + :aria-checked="ratioLabel === ratio" + :tabindex="ratioLabel === ratio ? 0 : -1" + class="flex-1 px-3 py-1.5 font-medium whitespace-nowrap sm:flex-none" + :class=" + ratioLabel === ratio + ? 'bg-primary text-inverted' + : 'bg-transparent text-toned' + " + @click="selectRatio(ratio)" > - {{ dimsLabel }} - </span> + {{ ratio }} + </button> </div> </div> - </div> - <div class="flex flex-col gap-1.5"> - <div class="flex items-center gap-1"> - <span class="flex-1 font-medium text-highlighted">Aspect ratio</span> - <UButton - color="neutral" - variant="ghost" - size="sm" - icon="i-lucide-rotate-ccw-square" - @click="rotateImage" + <div class="flex flex-col gap-1.5"> + <span class="font-medium text-highlighted">Width</span> + <div + role="radiogroup" + aria-label="Width" + class="flex w-full border border-accented sm:w-max" > - Rotate - </UButton> - <UButton - color="neutral" - variant="ghost" - size="sm" - icon="i-lucide-undo-2" - @click="resetCrop" - > - Reset - </UButton> + <button + v-for="widthOption in WIDTH_PRESETS" + :key="widthOption" + type="button" + role="radio" + :aria-checked="widthLabel === widthOption" + :tabindex="widthLabel === widthOption ? 0 : -1" + class="flex-1 px-3 py-1.5 font-medium whitespace-nowrap sm:flex-none" + :class=" + widthLabel === widthOption + ? 'bg-primary text-inverted' + : 'bg-transparent text-toned' + " + @click="selectWidth(widthOption)" + > + {{ widthOption }} + </button> + </div> + <span class="font-mono text-xs text-muted">{{ monoLine }}</span> </div> + <div - role="radiogroup" - aria-label="Aspect ratio" - class="flex w-full border border-accented sm:w-max" + v-if="showNarrowNote" + role="status" + class="flex gap-2 bg-(--rc-info-soft) p-2.5" > - <button - v-for="ratio in ASPECT_RATIOS" - :key="ratio" - type="button" - role="radio" - :aria-checked="ratioLabel === ratio" - :tabindex="ratioLabel === ratio ? 0 : -1" - class="flex-1 px-3 py-1.5 font-medium whitespace-nowrap sm:flex-none" - :class=" - ratioLabel === ratio - ? 'bg-primary text-inverted' - : 'bg-transparent text-toned' - " - @click="selectRatio(ratio)" - > - {{ ratio }} - </button> + <UIcon name="i-lucide-info" class="mt-0.5 size-4 shrink-0 text-info" /> + <span class="text-toned">{{ narrowNoteText }}</span> </div> </div> - <div class="flex flex-col gap-1.5"> - <span class="font-medium text-highlighted">Width</span> + <div v-else-if="phase === 'uploading'" class="flex flex-col gap-3"> <div - role="radiogroup" - aria-label="Width" - class="flex w-full border border-accented sm:w-max" + class="flex flex-col items-center gap-3 rounded-md border border-default px-6 py-8 text-center" > - <button - v-for="widthOption in WIDTH_PRESETS" - :key="widthOption" - type="button" - role="radio" - :aria-checked="widthLabel === widthOption" - :tabindex="widthLabel === widthOption ? 0 : -1" - class="flex-1 px-3 py-1.5 font-medium whitespace-nowrap sm:flex-none" - :class=" - widthLabel === widthOption - ? 'bg-primary text-inverted' - : 'bg-transparent text-toned' - " - @click="selectWidth(widthOption)" - > - {{ widthOption }} - </button> + <UIcon + :name="succeeded ? 'i-lucide-circle-check' : 'i-lucide-image-plus'" + :class="succeeded ? 'size-6 text-success' : 'size-6 text-dimmed'" + /> + <p class="text-sm text-highlighted"> + <template v-if="succeeded">Image added</template> + <template v-else>{{ chosenFile?.name }} {{ uploadPercent }}%</template> + </p> + <UProgress + v-if="!succeeded" + :model-value="uploadPercent" + size="xs" + class="w-full" + role="progressbar" + :aria-valuenow="uploadPercent" + /> </div> - <span class="font-mono text-xs text-muted">{{ monoLine }}</span> - </div> - - <div - v-if="showNarrowNote" - role="status" - class="flex gap-2 bg-(--rc-info-soft) p-2.5" - > - <UIcon name="i-lucide-info" class="mt-0.5 size-4 shrink-0 text-info" /> - <span class="text-toned">{{ narrowNoteText }}</span> - </div> - </div> - - <div v-else-if="phase === 'uploading'" class="flex flex-col gap-3"> - <div - class="flex flex-col items-center gap-3 rounded-md border border-default px-6 py-8 text-center" - > - <UIcon - :name="succeeded ? 'i-lucide-circle-check' : 'i-lucide-image-plus'" - :class="succeeded ? 'size-6 text-success' : 'size-6 text-dimmed'" - /> - <p class="text-sm text-highlighted"> - <template v-if="succeeded">Image added</template> - <template v-else>{{ chosenFile?.name }} {{ uploadPercent }}%</template> + <p v-if="!succeeded" class="text-sm text-toned"> + {{ formatUploadProgress(progress.loaded, progress.total) }} </p> - <UProgress - v-if="!succeeded" - :model-value="uploadPercent" - size="xs" - class="w-full" - role="progressbar" - :aria-valuenow="uploadPercent" - /> </div> - <p v-if="!succeeded" class="text-sm text-toned"> - {{ formatUploadProgress(progress.loaded, progress.total) }} - </p> - </div> - <div v-else-if="phase === 'rejected'" class="flex flex-col gap-1"> - <div - class="flex flex-col items-center gap-3 rounded-md border border-error px-6 py-8 text-center" - > - <UIcon name="i-lucide-file-x" class="size-6 text-error" /> - <p class="text-sm font-semibold text-highlighted">{{ rejectedTitle }}</p> - <p class="text-sm text-toned">{{ rejectedText }}</p> + <div v-else-if="phase === 'rejected'" class="flex flex-col gap-1"> + <div + class="flex flex-col items-center gap-3 rounded-md border border-error px-6 py-8 text-center" + > + <UIcon name="i-lucide-file-x" class="size-6 text-error" /> + <p class="text-sm font-semibold text-highlighted">{{ rejectedTitle }}</p> + <p class="text-sm text-toned">{{ rejectedText }}</p> + </div> </div> - </div> - <div v-else-if="phase === 'failed'" class="flex flex-col gap-1"> - <p class="text-sm font-semibold text-highlighted">Upload failed</p> - <p class="text-sm text-toned"> - The server didn't accept the file. Your article hasn't changed. - </p> + <div v-else-if="phase === 'failed'" class="flex flex-col gap-1"> + <p class="text-sm font-semibold text-highlighted">Upload failed</p> + <p class="text-sm text-toned"> + The server didn't accept the file. Your article hasn't changed. + </p> + </div> </div> </template> <template #footer> <div class="flex w-full flex-col-reverse gap-2 sm:flex-row sm:justify-end"> - <UButton v-if="showCancel" color="neutral" variant="outline" @click="cancel"> + <UButton + v-if="showCancel" + color="neutral" + variant="outline" + block + :class="FOOTER_BUTTON_CLASS" + class="text-highlighted" + @click="cancel" + > {{ cancelLabel }} </UButton> - <UButton v-if="phase === 'rejected'" @click="chooseAnotherFile"> + <UButton + v-if="onLibraryTab" + block + :class="FOOTER_BUTTON_CLASS" + :disabled="!selected" + @click="insertSelected" + > + Insert image + </UButton> + <UButton + v-if="showRejectedActions" + block + :class="FOOTER_BUTTON_CLASS" + @click="chooseAnotherFile" + > Choose another file </UButton> - <UButton v-if="phase === 'failed'" @click="tryAgain"> Try again </UButton> - <template v-if="phase === 'crop'"> - <UButton color="neutral" variant="outline" @click="chooseAnotherFile"> + <UButton + v-if="showFailedActions" + block + :class="FOOTER_BUTTON_CLASS" + @click="tryAgain" + > + Try again + </UButton> + <template v-if="showCropActions"> + <UButton + color="neutral" + variant="outline" + block + :class="FOOTER_BUTTON_CLASS" + class="text-highlighted" + @click="chooseAnotherFile" + > Choose another file </UButton> - <UButton @click="confirmCropUpload">Upload image</UButton> + <UButton block :class="FOOTER_BUTTON_CLASS" @click="confirmCropUpload"> + Upload image + </UButton> </template> </div> </template> diff --git a/web/src/components/MediaPicker.vue b/web/src/components/MediaPicker.vue new file mode 100644 index 0000000..b88e081 --- /dev/null +++ b/web/src/components/MediaPicker.vue @@ -0,0 +1,337 @@ +<script setup lang="ts"> +import { computed, nextTick, onBeforeUnmount, ref, watch } from 'vue' +import { useMediaQuery } from '@vueuse/core' +import MediaThumb from '@/components/MediaThumb.vue' +import type { LibraryStatus } from '@/composables/useLibraryList' +import type { MediaItem } from '@/composables/useMedia' +import { accessibleName, formatDimensions, formatSize } from '@/lib/mediaFormat' + +const SEARCH_DEBOUNCE_MS = 120 +const DESKTOP_QUERY = '(min-width: 48rem)' +const FIRST_LOAD_SKELETONS_DESKTOP = 8 +const FIRST_LOAD_SKELETONS_MOBILE = 9 +const COLUMNS_DESKTOP = 4 +const COLUMNS_MOBILE = 3 +const SEARCH_ICON_SIZE = 'size-3.5' +const EMPTY_STATE_UI = { + root: 'gap-3 border border-default bg-default px-6 py-8', + header: 'gap-3', + avatar: 'mb-0 size-11 rounded-none bg-elevated text-toned', + title: 'text-[0.9375rem] font-semibold', + description: 'max-w-80 text-muted text-pretty', +} + +const props = defineProps<{ + items: MediaItem[] + status: LibraryStatus + showSkeleton: boolean + loadingMore: boolean + hasMore: boolean + total: number + query: string + firstNewId: number | null + inArticleIds: number[] + selected: MediaItem | null +}>() + +const emit = defineEmits<{ + search: [query: string] + select: [item: MediaItem] + 'load-more': [] + retry: [] + upload: [] +}>() + +const alt = defineModel<string>('alt', { default: '' }) + +const isDesktop = useMediaQuery(DESKTOP_QUERY) +const grid = ref<HTMLElement | null>(null) +const searchText = ref(props.query) +let debounceTimer: ReturnType<typeof setTimeout> | null = null + +const columns = computed(() => (isDesktop.value ? COLUMNS_DESKTOP : COLUMNS_MOBILE)) +const firstLoadSkeletons = computed(() => + isDesktop.value ? FIRST_LOAD_SKELETONS_DESKTOP : FIRST_LOAD_SKELETONS_MOBILE, +) +const searching = computed(() => props.query !== '') +const summary = computed(() => + searching.value ? 'Newest first' : `Newest first · ${props.total} images`, +) +const scrollHeight = computed(() => (props.selected ? 'max-h-32 md:max-h-49.5' : 'max-h-77')) +const showEmptyLibrary = computed( + () => props.status === 'ready' && props.items.length === 0 && !searching.value, +) +const showNoResults = computed( + () => props.status === 'ready' && props.items.length === 0 && searching.value, +) +const showGrid = computed(() => props.status === 'ready' && props.items.length > 0) +const showLoadMore = computed(() => showGrid.value && props.hasMore && !props.selected) +const noResultsTitle = computed(() => `No images match "${props.query}"`) + +function clearDebounce() { + if (debounceTimer !== null) { + clearTimeout(debounceTimer) + debounceTimer = null + } +} + +watch(searchText, (value) => { + clearDebounce() + debounceTimer = setTimeout(() => { + debounceTimer = null + emit('search', value.trim()) + }, SEARCH_DEBOUNCE_MS) +}) + +function clearSearch() { + clearDebounce() + searchText.value = '' + emit('search', '') +} + +function isSelected(item: MediaItem) { + return props.selected?.id === item.id +} + +function isInArticle(item: MediaItem) { + return props.inArticleIds.includes(item.id) +} + +function onTileKeydown(event: KeyboardEvent, item: MediaItem) { + if (event.key !== 'Enter' && event.key !== ' ') { + return + } + event.preventDefault() + if (!event.repeat) { + emit('select', item) + } +} + +function onTileKeyup(event: KeyboardEvent) { + if (event.key === ' ') { + event.preventDefault() + } +} + +watch( + () => props.firstNewId, + async (id) => { + if (id === null) { + return + } + await nextTick() + grid.value?.querySelector<HTMLElement>(`[data-image-id="${id}"]`)?.focus() + }, +) + +onBeforeUnmount(clearDebounce) +</script> + +<template> + <div class="flex min-h-0 flex-col gap-3"> + <UInput + v-model="searchText" + type="text" + icon="i-lucide-search" + placeholder="Search by filename" + aria-label="Search by filename" + class="w-full" + :ui="{ base: 'h-11 text-lg md:h-8 md:text-sm', leadingIcon: SEARCH_ICON_SIZE }" + > + <template v-if="searchText" #trailing> + <UButton + color="neutral" + variant="link" + size="xs" + icon="i-lucide-x" + aria-label="Clear search" + class="text-muted" + @click="clearSearch" + /> + </template> + </UInput> + + <UAlert + v-if="status === 'error'" + role="alert" + color="error" + variant="soft" + orientation="vertical" + :close="false" + icon="i-lucide-circle-x" + title="Couldn't load your library" + description="Check your connection and try again." + :ui="{ actions: 'mt-2' }" + > + <template #actions> + <UButton + color="neutral" + variant="outline" + icon="i-lucide-rotate-cw" + class="h-11 gap-1.5 border border-accented bg-default px-2.5 text-xs text-highlighted ring-0 md:h-7" + :ui="{ leadingIcon: 'size-3.5' }" + @click="emit('retry')" + > + Try again + </UButton> + </template> + </UAlert> + + <UEmpty + v-else-if="showEmptyLibrary" + variant="naked" + icon="i-lucide-images" + title="Your library is empty" + description="Images you upload appear here, ready to use in any article." + :ui="EMPTY_STATE_UI" + > + <template #actions> + <UButton icon="i-lucide-upload" class="h-11 px-4 md:h-8" @click="emit('upload')"> + Upload an image + </UButton> + </template> + </UEmpty> + + <UEmpty + v-else-if="showNoResults" + variant="naked" + icon="i-lucide-search" + :title="noResultsTitle" + description="Check the spelling, or search for a shorter part of the filename." + :ui="EMPTY_STATE_UI" + > + <template #actions> + <UButton + color="neutral" + variant="outline" + class="h-11 border border-accented bg-default px-4 text-highlighted ring-0 md:h-8" + @click="clearSearch" + > + Clear search + </UButton> + </template> + </UEmpty> + + <div + v-else-if="status === 'loading' && showSkeleton" + aria-busy="true" + class="grid grid-cols-3 gap-x-2 gap-y-3 md:grid-cols-4 md:gap-x-3" + > + <div + v-for="n in firstLoadSkeletons" + :key="n" + data-testid="picker-skeleton-tile" + class="flex flex-col gap-1.5" + > + <USkeleton class="aspect-square w-full rounded-none bg-accented" /> + <USkeleton class="h-2.5 w-[70%] rounded-none bg-accented" /> + </div> + </div> + + <template v-else-if="showGrid"> + <span data-testid="picker-summary" class="text-xs/5 text-muted">{{ summary }}</span> + <div class="-m-1 min-h-0 overflow-y-auto p-1" :class="scrollHeight"> + <div ref="grid" class="grid grid-cols-3 gap-x-2 gap-y-3 md:grid-cols-4 md:gap-x-3"> + <button + v-for="item in items" + :key="item.id" + type="button" + data-picker-tile + :data-image-id="item.id" + :aria-label="accessibleName(item)" + :aria-pressed="isSelected(item)" + class="group flex min-w-0 flex-col gap-1.5 text-start focus-visible:outline-none" + @click="emit('select', item)" + @keydown="onTileKeydown($event, item)" + @keyup="onTileKeyup" + > + <span + class="relative block w-full group-focus-visible:outline-2 group-focus-visible:outline-offset-2 group-focus-visible:outline-primary" + :class=" + isSelected(item) ? 'outline-2 outline-offset-2 outline-primary' : '' + " + > + <MediaThumb + :item="item" + size="md" + class="aspect-square w-full border border-default" + /> + <span + v-if="isInArticle(item)" + class="absolute bottom-1 left-1 inline-flex h-4.5 items-center bg-secondary-200 px-1.25 text-[0.6875rem] font-medium text-primary-900 dark:bg-secondary-300" + >In this article</span + > + <span + v-if="isSelected(item)" + data-testid="picker-check" + class="absolute top-1 right-1 flex size-5 items-center justify-center bg-primary text-inverted" + > + <UIcon name="i-lucide-check" class="size-3.5" /> + </span> + </span> + <span class="truncate text-xs/5 text-toned">{{ item.filename }}</span> + </button> + <template v-if="loadingMore && showSkeleton"> + <div + v-for="n in columns" + :key="`skeleton-${n}`" + data-testid="picker-skeleton-tile" + class="flex flex-col gap-1.5" + > + <USkeleton class="aspect-square w-full rounded-none bg-accented" /> + <USkeleton class="h-2.5 w-[70%] rounded-none bg-accented" /> + </div> + </template> + </div> + </div> + <div v-if="showLoadMore" class="flex justify-center"> + <UButton + color="neutral" + variant="outline" + :icon="loadingMore ? 'i-lucide-loader-circle' : undefined" + :aria-disabled="loadingMore || undefined" + class="h-11 border border-accented bg-default px-3 text-highlighted ring-0 aria-disabled:opacity-40 md:h-8" + :ui="{ leadingIcon: loadingMore ? 'size-4 animate-spin' : 'size-4' }" + @click="emit('load-more')" + > + {{ loadingMore ? 'Loading…' : 'Load more' }} + </UButton> + </div> + </template> + + <div + v-if="selected" + data-testid="picker-panel" + class="flex flex-col gap-3 border-t border-default pt-3" + > + <div class="flex items-center gap-3"> + <MediaThumb :item="selected" size="sm" class="size-14 border border-default" /> + <div class="flex min-w-0 flex-col"> + <span class="truncate font-medium text-highlighted">{{ + selected.filename + }}</span> + <span class="text-xs/5 text-muted" + >{{ formatDimensions(selected.width, selected.height) }} · + {{ formatSize(selected.size) }}</span + > + </div> + </div> + <UFormField + label="Alt text (optional)" + help="Describes the image for people who use a screen reader, and shows if it doesn't load. Leave it empty if the image is only decoration." + :ui="{ + label: 'font-medium text-highlighted', + help: 'text-xs/5 text-muted', + }" + > + <UInput + v-model="alt" + type="text" + placeholder="e.g. North wall of the kitchen before plastering" + class="w-full" + :ui="{ base: 'h-11 text-lg md:h-10 md:text-sm' }" + /> + </UFormField> + </div> + </div> +</template> diff --git a/web/src/components/MediaToolbar.vue b/web/src/components/MediaToolbar.vue index 3e60d64..287cc23 100644 --- a/web/src/components/MediaToolbar.vue +++ b/web/src/components/MediaToolbar.vue @@ -2,12 +2,10 @@ import { computed, onBeforeUnmount, ref, watch } from 'vue' import type { MediaCounts } from '@/composables/useMedia' import type { MediaControls, MediaFilter, MediaSort, MediaView } from '@/lib/mediaQuery' +import { UNDERLINE_TAB_TRIGGER } from '@/lib/tabStyles' const SEARCH_DEBOUNCE_MS = 120 -const TAB_TRIGGER = - 'h-9 px-0 py-0 data-[state=inactive]:text-toned in-[[data-slot=list]:not(:has([data-slot=indicator]))]:data-[state=active]:after:bottom-0 in-[[data-slot=list]:not(:has([data-slot=indicator]))]:data-[state=active]:after:h-0.5 max-md:h-11 max-md:text-[0.9375rem]' - const SORT_LABELS: Record<MediaSort, string> = { newest: 'Newest', oldest: 'Oldest', @@ -107,7 +105,7 @@ const sortItems = computed(() => class="order-2 md:order-none md:-mb-px md:flex-1" :ui="{ list: 'gap-5 border-b-0 p-0 max-md:border-b', - trigger: TAB_TRIGGER, + trigger: UNDERLINE_TAB_TRIGGER, }" > <template #default="{ item }"> diff --git a/web/src/components/__tests__/ArticleDetails.spec.ts b/web/src/components/__tests__/ArticleDetails.spec.ts new file mode 100644 index 0000000..0eda836 --- /dev/null +++ b/web/src/components/__tests__/ArticleDetails.spec.ts @@ -0,0 +1,46 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mount } from '@vue/test-utils' +import uiPlugin from '@nuxt/ui/vue-plugin' +import ArticleDetails from '../ArticleDetails.vue' +import type { Article } from '@/composables/useArticles' + +afterEach(() => { + document.body.innerHTML = '' +}) + +function mountDetails(article: Omit<Article, 'images'>) { + return mount(ArticleDetails, { + props: { article: article as Article }, + global: { plugins: [uiPlugin] }, + attachTo: document.body, + }) +} + +const baseArticle = { + id: 3, + title: 'Kitchen', + body: '<p>a</p>', + is_online: false, + created_at: '2026-03-12T09:14:00Z', + updated_at: '2026-09-26T11:02:00Z', +} + +describe('An article from the API carries no images field', () => { + it('renders the details with an images count read from the body', () => { + const wrapper = mountDetails({ + ...baseArticle, + body: '<p>a</p><img src="/a.png"><img src="/b.png">', + }) + + expect(wrapper.text()).toContain('Images') + expect(wrapper.text()).toContain('2') + wrapper.unmount() + }) + + it('counts no image for a body without any', () => { + const wrapper = mountDetails(baseArticle) + + expect(wrapper.text()).toContain('Images') + wrapper.unmount() + }) +}) diff --git a/web/src/components/__tests__/ArticleEditor.spec.ts b/web/src/components/__tests__/ArticleEditor.spec.ts index 04da746..83635c2 100644 --- a/web/src/components/__tests__/ArticleEditor.spec.ts +++ b/web/src/components/__tests__/ArticleEditor.spec.ts @@ -34,7 +34,6 @@ async function mountArticleEditor(body = '<p>Hello</p>') { id: 12, title: 'T', body, - images: [], is_online: false, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', @@ -106,3 +105,17 @@ describe('removeImage removes only the node whose data-image-id matches', () => expect(editor.emitted('update:modelValue')).toBeFalsy() }) }) + +describe('imageSrc reads the src of the node whose data-image-id matches', () => { + it('returns the src of the matching image and null for an unknown id', async () => { + const wrapper = await mountArticleEditor( + '<p>Hello</p><img src="/static/uploadedImages/a.png" alt="" data-image-id="10">', + ) + const vm = wrapper.findComponent(ArticleEditor).vm as unknown as { + imageSrc: (id: number) => string | null + } + + expect(vm.imageSrc(10)).toBe('/static/uploadedImages/a.png') + expect(vm.imageSrc(999)).toBeNull() + }) +}) diff --git a/web/src/components/__tests__/ConfirmDialog.spec.ts b/web/src/components/__tests__/ConfirmDialog.spec.ts index c05edca..eb32d3c 100644 --- a/web/src/components/__tests__/ConfirmDialog.spec.ts +++ b/web/src/components/__tests__/ConfirmDialog.spec.ts @@ -74,14 +74,9 @@ describe('Cancelling — the action is in flight and the dialog cannot be dismis await clickButton('Cancel registration') expect(action).toHaveBeenCalledTimes(1) - expect( - (findButtonElement('Keep my account') as HTMLButtonElement) - .disabled, - ).toBe(true) + expect((findButtonElement('Keep my account') as HTMLButtonElement).disabled).toBe(true) expect(findButtonElement('Cancelling…')).toBeTruthy() - expect( - (findButtonElement('Cancelling…') as HTMLButtonElement).disabled, - ).toBe(true) + expect((findButtonElement('Cancelling…') as HTMLButtonElement).disabled).toBe(true) expect(document.querySelector('[aria-busy="true"]')).toBeTruthy() resolveAction?.() @@ -163,26 +158,60 @@ describe('dismissResolvesTo="confirm" makes Esc/overlay resolve as confirmed', ( describe('Cancel failed — the dialog stays open and shows the failure inline', () => { it('re-enables both buttons and shows the rejection message inline', async () => { - const action = vi.fn(() => - Promise.reject(new Error("Couldn't cancel registration.")), - ) + const action = vi.fn(() => Promise.reject(new Error("Couldn't cancel registration."))) const wrapper = mountDialog(action) await flushPromises() await clickButton('Cancel registration') expect(wrapper.emitted('close')).toBeFalsy() - expect(document.body.textContent).toContain( - "Couldn't cancel registration.", - ) - expect( - (findButtonElement('Keep my account') as HTMLButtonElement) - .disabled, - ).toBe(false) - expect( - (findButtonElement('Cancel registration') as HTMLButtonElement) - .disabled, - ).toBe(false) + expect(document.body.textContent).toContain("Couldn't cancel registration.") + expect((findButtonElement('Keep my account') as HTMLButtonElement).disabled).toBe(false) + expect((findButtonElement('Cancel registration') as HTMLButtonElement).disabled).toBe(false) + wrapper.unmount() + }) +}) + +describe('A neutral confirm with an image preview', () => { + function mountPreview(preview: { src: string; filename?: string; size?: string }) { + return mount(ConfirmDialog, { + attachTo: document.body, + props: { + title: 'Remove this image from the article?', + description: 'It stays in your library.', + confirmLabel: 'Remove image', + cancelLabel: 'Cancel', + pendingLabel: 'Removing…', + tone: 'neutral', + preview, + action: async () => {}, + }, + global: { plugins: [uiPlugin] }, + }) + } + + it('shows the thumbnail, the filename and the size, and a primary confirm', async () => { + const wrapper = mountPreview({ + src: '/media/north-wall.jpg', + filename: 'north-wall.jpg', + size: '3.9 MB', + }) + await flushPromises() + + const row = document.body.querySelector('[data-testid="confirm-preview"]')! + expect(row.textContent).toContain('north-wall.jpg') + expect(row.textContent).toContain('Stays in the library · 3.9 MB') + expect(row.querySelector('img')?.getAttribute('src')).toBe('/media/north-wall.jpg') + expect(findButtonElement('Remove image')?.className).not.toContain('bg-error') + wrapper.unmount() + }) + + it('shows the thumbnail and "Stays in the library" for an image the session has not seen', async () => { + const wrapper = mountPreview({ src: '/media/seven.jpg' }) + await flushPromises() + + const row = document.body.querySelector('[data-testid="confirm-preview"]')! + expect(row.textContent?.replace(/\s+/g, ' ').trim()).toBe('Stays in the library') wrapper.unmount() }) }) diff --git a/web/src/components/__tests__/ImageUploader.spec.ts b/web/src/components/__tests__/ImageUploader.spec.ts index 055b40d..cad0606 100644 --- a/web/src/components/__tests__/ImageUploader.spec.ts +++ b/web/src/components/__tests__/ImageUploader.spec.ts @@ -1,16 +1,44 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { defineComponent, nextTick, ref } from 'vue' -import { flushPromises, mount } from '@vue/test-utils' -import uiPlugin from '@nuxt/ui/vue-plugin' -import ImageUploader from '../ImageUploader.vue' +import { nextTick } from 'vue' +import * as fileHash from '@/lib/fileHash' import * as imageUpload from '@/lib/imageUpload' +import { ApiError, NetworkError, redirectToLogin, redirectUnverified } from '@/composables/useApi' +import { fixture } from '@/views/__tests__/mediaTestKit' +import { + altField, + apiKit, + click, + dialogText, + findButton, + hashCalls, + installApi, + installEmptyLibrary, + listCalls, + modalOpen, + mountUploader, + openUploader, + pageBody, + selectedTab, + settle, + tabButtons, + textOf, +} from './pickerKit' + +vi.mock('@/composables/useApi', async (importOriginal) => { + const actual = await importOriginal<typeof import('@/composables/useApi')>() + return { + ...actual, + useApi: vi.fn(), + redirectToLogin: vi.fn(), + redirectUnverified: vi.fn(), + } +}) + +vi.mock('@/lib/fileHash', () => ({ hashFile: vi.fn() })) vi.mock('@/lib/imageUpload', async (importOriginal) => { const actual = await importOriginal<typeof import('@/lib/imageUpload')>() - return { - ...actual, - validateImageFile: vi.fn(actual.validateImageFile), - } + return { ...actual, validateImageFile: vi.fn(actual.validateImageFile) } }) vi.mock('@/lib/imageCrop', async (importOriginal) => { @@ -22,6 +50,8 @@ vi.mock('@/lib/imageCrop', async (importOriginal) => { } }) +const NORTH_WALL_HASH = 'a'.repeat(64) + class FakeXMLHttpRequest { static instances: FakeXMLHttpRequest[] = [] @@ -88,7 +118,7 @@ class FakeXMLHttpRequest { } function latestXHR() { - return FakeXMLHttpRequest.instances[FakeXMLHttpRequest.instances.length - 1] + return FakeXMLHttpRequest.instances[FakeXMLHttpRequest.instances.length - 1]! } function mib(value: number) { @@ -101,38 +131,6 @@ function fileFixture(name: string, type: string, sizeMB: number) { return file } -const Host = defineComponent({ - components: { ImageUploader }, - props: { - articleId: { type: Number, required: true }, - initialFile: { type: Object as () => File | null, default: null }, - }, - emits: ['uploaded', 'close'], - setup(_props, { emit }) { - const open = ref(false) - return { - open, - onUploaded: (location: string, imageId: number) => emit('uploaded', location, imageId), - onClose: () => emit('close'), - } - }, - template: `<ImageUploader v-model:open="open" :article-id="articleId" :initial-file="initialFile" @uploaded="onUploaded" @close="onClose" />`, -}) - -function mountUploader(props: { articleId?: number; initialFile?: File | null } = {}) { - return mount(Host, { - props: { articleId: 12, initialFile: null, ...props }, - global: { plugins: [uiPlugin] }, - attachTo: document.body, - }) -} - -async function openModal(wrapper: ReturnType<typeof mountUploader>) { - ;(wrapper.vm as unknown as { open: boolean }).open = true - await nextTick() - await nextTick() -} - function fileInput() { return document.body.querySelector<HTMLInputElement>('[data-testid="image-file-input"]')! } @@ -149,21 +147,11 @@ async function chooseFile(file: File) { }) input.dispatchEvent(new Event('change')) await nextTick() - await nextTick() - await flushPromises() -} - -function findButton(text: string) { - return Array.from(document.body.querySelectorAll('button')).find( - (button) => button.textContent?.trim() === text, - ) + await settle() } async function clickButton(text: string) { - findButton(text)?.click() - await nextTick() - await nextTick() - await flushPromises() + await click(findButton(text)) } async function chooseRasterFileAndConfirmUpload(file: File) { @@ -171,47 +159,78 @@ async function chooseRasterFileAndConfirmUpload(file: File) { await clickButton('Upload image') } +function mediaItem(overrides: Record<string, unknown> = {}) { + return { ...fixture([1])[0]!, ...overrides } +} + +let wrapper: ReturnType<typeof mountUploader> | null = null + +async function openOnUpload(routes: Parameters<typeof installEmptyLibrary>[0] = {}) { + installEmptyLibrary(routes) + wrapper = mountUploader() + await openUploader(wrapper) +} + const RealXMLHttpRequest = global.XMLHttpRequest beforeEach(() => { FakeXMLHttpRequest.instances = [] global.XMLHttpRequest = FakeXMLHttpRequest as unknown as typeof XMLHttpRequest + vi.mocked(fileHash.hashFile).mockResolvedValue(NORTH_WALL_HASH) + vi.mocked(redirectToLogin).mockClear() + vi.mocked(redirectUnverified).mockClear() }) afterEach(() => { + wrapper?.unmount() + wrapper = null + vi.useRealTimers() document.body.innerHTML = '' global.XMLHttpRequest = RealXMLHttpRequest }) -describe('Opening the uploader from the toolbar image button shows the idle state', () => { - it('shows the idle copy and sends no request', async () => { - const wrapper = mountUploader() - await openModal(wrapper) +describe("The toolbar's image button opens on Upload when the library is empty", () => { + it('shows both tabs, the drop zone copy and a footer with Cancel only', async () => { + await openOnUpload() - expect(document.body.textContent).toContain('Add an image') - expect(document.body.textContent).toContain('Drop an image here') - expect(document.body.textContent).toContain('PNG, JPEG, WEBP, SVG · max 10 MB') - expect(findButton('Choose file')).toBeTruthy() + expect(dialogText()).toContain('Add an image') + expect(tabButtons().map(textOf)).toEqual(['Upload', 'Library']) + expect(textOf(selectedTab())).toBe('Upload') + expect(dialogText()).toContain('Drop an image here, or choose a file') + expect(dialogText()).toContain('PNG, JPEG, WEBP, SVG · max 10 MB') + expect(findButton('Choose file')).toBeUndefined() expect(findButton('Cancel')).toBeTruthy() + expect(findButton('Insert image')).toBeUndefined() expect(FakeXMLHttpRequest.instances).toHaveLength(0) - wrapper.unmount() }) }) describe('Dragging a file over the drop zone shows the dragging-over state', () => { it('shows "Drop to upload"', async () => { - const wrapper = mountUploader() - await openModal(wrapper) + await openOnUpload() dropZone().dispatchEvent(new Event('dragover', { bubbles: true, cancelable: true })) await nextTick() - expect(document.body.textContent).toContain('Drop to upload') - wrapper.unmount() + expect(dialogText()).toContain('Drop to upload') }) }) -describe('Choosing a file the browser can reject never sends a request', () => { +describe('Dropping a file opens Upload with the file', () => { + it('looks the hash up, sends no list request and opens the crop step', async () => { + installApi() + wrapper = mountUploader({ initialFile: fileFixture('before.png', 'image/png', 3.9) }) + await openUploader(wrapper) + + expect(hashCalls()).toEqual([NORTH_WALL_HASH]) + expect(listCalls()).toHaveLength(0) + expect(dialogText()).toContain('Crop and resize') + expect(dialogText()).toContain('before.png') + expect(FakeXMLHttpRequest.instances).toHaveLength(0) + }) +}) + +describe('Choosing a file the browser can reject sends nothing', () => { it.each([ { filename: 'plan.pdf', @@ -234,183 +253,368 @@ describe('Choosing a file the browser can reject never sends a request', () => { ])( 'rejects $filename with the exact copy', async ({ filename, contentType, sizeMB, description }) => { - const wrapper = mountUploader() - await openModal(wrapper) + await openOnUpload() await chooseFile(fileFixture(filename, contentType, sizeMB)) + expect(hashCalls()).toHaveLength(0) expect(FakeXMLHttpRequest.instances).toHaveLength(0) - expect(document.body.textContent).toContain(`"${filename}" can't be used`) - expect(document.body.textContent).toContain(description) + expect(dialogText()).toContain(`"${filename}" can't be used`) + expect(dialogText()).toContain(description) expect(findButton('Choose another file')).toBeTruthy() expect(findButton('Cancel')).toBeTruthy() - wrapper.unmount() + expect(tabButtons()).toHaveLength(0) }, ) }) describe('"Choose another file" from the rejected state returns to idle', () => { - it('shows the idle state again', async () => { - const wrapper = mountUploader() - await openModal(wrapper) + it('shows the idle state again, with the tabs', async () => { + await openOnUpload() await chooseFile(fileFixture('plan.pdf', 'application/pdf', 1.2)) await clickButton('Choose another file') - expect(document.body.textContent).toContain('Drop an image here') - wrapper.unmount() + expect(dialogText()).toContain('Drop an image here, or choose a file') + expect(tabButtons().map(textOf)).toEqual(['Upload', 'Library']) + }) +}) + +describe('A file whose content exists shows the duplicate card and sends nothing', () => { + it('shows the card with the oldest match and its two actions', async () => { + await openOnUpload({ hash: () => pageBody([mediaItem()], null) }) + + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + const card = textOf(document.body.querySelector('[data-testid="duplicate-card"]')) + expect(card).toContain('Already in your library') + expect(card).toContain( + 'north-wall.jpg matches an image uploaded on 12 Sep 2026 by Marta Alves.', + ) + expect(findButton('Use existing')).toBeTruthy() + expect(findButton('Upload anyway')).toBeTruthy() + expect(FakeXMLHttpRequest.instances).toHaveLength(0) + expect(dialogText()).not.toContain('Crop and resize') + expect(tabButtons()).toHaveLength(2) + }) + + it('names the oldest of several matches', async () => { + await openOnUpload({ + hash: () => + pageBody( + [ + mediaItem({ + id: 9, + created_at: '2026-09-20T12:00:00Z', + uploaded_by: 'jonas', + }), + mediaItem({ + id: 3, + created_at: '2026-09-01T12:00:00Z', + uploaded_by: 'Inès Duarte', + }), + ], + null, + ), + }) + + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + expect(dialogText()).toContain( + 'north-wall.jpg matches an image uploaded on 1 Sep 2026 by Inès Duarte.', + ) + }) + + it('drops the "by" part when the uploader is unknown', async () => { + await openOnUpload({ hash: () => pageBody([mediaItem({ uploaded_by: '' })], null) }) + + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + expect(dialogText()).toContain('north-wall.jpg matches an image uploaded on 12 Sep 2026.') + }) + + it('goes to the Library with the existing image selected when "Use existing" is activated', async () => { + await openOnUpload({ hash: () => pageBody([mediaItem()], null) }) + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + await clickButton('Use existing') + + expect(textOf(selectedTab())).toBe('Library') + expect(textOf(document.body.querySelector('[data-testid="picker-panel"]'))).toContain( + 'north-wall.jpg', + ) + expect(altField()!.value).toBe('') + expect(findButton('Insert image')?.disabled).toBe(false) + expect(FakeXMLHttpRequest.instances).toHaveLength(0) + }) + + it('continues to crop and upload without a second lookup when "Upload anyway" is activated', async () => { + await openOnUpload({ hash: () => pageBody([mediaItem()], null) }) + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + await clickButton('Upload anyway') + + expect(dialogText()).toContain('Crop and resize') + expect(hashCalls()).toHaveLength(1) + expect(FakeXMLHttpRequest.instances).toHaveLength(0) + }) + + it('uploads an SVG at once when "Upload anyway" is activated', async () => { + await openOnUpload({ hash: () => pageBody([mediaItem()], null) }) + await chooseFile(fileFixture('diagram.svg', 'image/svg+xml', 0.04)) + + await clickButton('Upload anyway') + + expect(FakeXMLHttpRequest.instances).toHaveLength(1) + expect(latestXHR().url).toBe('/v1/media/') + }) +}) + +describe('The duplicate check is skipped when it cannot run', () => { + it.each([ + { + name: 'crypto.subtle is undefined', + arrange: () => vi.mocked(fileHash.hashFile).mockResolvedValue(null), + routes: {}, + }, + { + name: 'the lookup resolves with a 500 response', + arrange: () => undefined, + routes: { hash: () => Promise.reject(new ApiError(500)) }, + }, + { + name: 'the lookup never reaches the server', + arrange: () => undefined, + routes: { hash: () => Promise.reject(new NetworkError()) }, + }, + ])('continues to the crop step with no card when $name', async ({ arrange, routes }) => { + arrange() + await openOnUpload(routes) + + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + expect(dialogText()).toContain('Crop and resize') + expect(document.body.querySelector('[data-testid="duplicate-card"]')).toBeNull() + }) + + it('uploads an SVG when the lookup fails', async () => { + await openOnUpload({ hash: () => Promise.reject(new NetworkError()) }) + + await chooseFile(fileFixture('diagram.svg', 'image/svg+xml', 0.04)) + + expect(FakeXMLHttpRequest.instances).toHaveLength(1) }) }) describe('A valid upload shows progress and its cancel action reads "Cancel upload"', () => { - it('sends the resized file and reports live progress', async () => { - const wrapper = mountUploader() - await openModal(wrapper) + it('sends the resized file to /v1/media/ and reports live progress', async () => { + await openOnUpload() await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) expect(FakeXMLHttpRequest.instances).toHaveLength(1) const xhr = latestXHR() - expect(xhr.url).toBe('/v1/article/12/image') + expect(xhr.method).toBe('POST') + expect(xhr.url).toBe('/v1/media/') + expect(xhr.withCredentials).toBe(true) expect(xhr.sentBody?.get('file')).toBeInstanceOf(File) - expect(document.body.textContent).toContain('north-wall.jpg 0%') - expect(document.body.textContent).toContain('Uploading 0.0 of 420 KB') + expect(dialogText()).toContain('north-wall.jpg 0%') + expect(dialogText()).toContain('Uploading 0.0 of 420 KB') expect(findButton('Cancel upload')).toBeTruthy() + expect(tabButtons()).toHaveLength(0) xhr.fireProgress(180 * 1024, 420 * 1024) await nextTick() - expect(document.body.textContent).toContain('north-wall.jpg 43%') - expect(document.body.textContent).toContain('Uploading 180 of 420 KB') - wrapper.unmount() + expect(dialogText()).toContain('north-wall.jpg 43%') + expect(dialogText()).toContain('Uploading 180 of 420 KB') + }) + + it('reads megabytes for a larger file', async () => { + await openOnUpload() + await chooseFile(fileFixture('logo.svg', 'image/svg+xml', 3.9)) + + latestXHR().fireProgress(mib(2.4), mib(3.9)) + await nextTick() + + expect(dialogText()).toContain('logo.svg 62%') + expect(dialogText()).toContain('Uploading 2.4 of 3.9 MB') }) }) describe('Closing the dialog during an upload cancels it', () => { it('aborts the in-flight XMLHttpRequest and closes the modal', async () => { - const wrapper = mountUploader() - await openModal(wrapper) + await openOnUpload() await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) const xhr = latestXHR() await clickButton('Cancel upload') expect(xhr.aborted).toBe(true) - expect(document.querySelector('[role="dialog"]')).toBeFalsy() - wrapper.unmount() + expect(modalOpen()).toBe(false) + expect(wrapper!.emitted('uploaded')).toBeUndefined() }) }) -describe('A successful upload emits the location and auto-closes 600 ms later', () => { +describe('A successful upload emits the library item and auto-closes 600 ms later', () => { it('shows "Image added" then closes', async () => { vi.useFakeTimers() - const wrapper = mountUploader() - await openModal(wrapper) - await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) - const xhr = latestXHR() + installEmptyLibrary() + wrapper = mountUploader() + ;(wrapper.vm as unknown as { open: boolean }).open = true + await vi.advanceTimersByTimeAsync(0) + await chooseFile(fileFixture('diagram.svg', 'image/svg+xml', 0.04)) + const item = mediaItem({ id: 13, url: '/static/uploadedImages/13.jpg' }) - xhr.resolve( - 201, - JSON.stringify({ id: 7, location: '/static/uploadedImages/north-wall.jpg' }), - ) + latestXHR().resolve(201, JSON.stringify(item)) await vi.advanceTimersByTimeAsync(0) - expect(wrapper.emitted('uploaded')).toEqual([['/static/uploadedImages/north-wall.jpg', 7]]) - expect(document.body.textContent).toContain('Image added') - expect(document.querySelector('[role="dialog"]')).toBeTruthy() + expect(wrapper.emitted('uploaded')).toEqual([[item]]) + expect(dialogText()).toContain('Image added') + expect(modalOpen()).toBe(true) await vi.advanceTimersByTimeAsync(590) - expect(document.querySelector('[role="dialog"]')).toBeTruthy() + expect(modalOpen()).toBe(true) await vi.advanceTimersByTimeAsync(10) - expect(document.querySelector('[role="dialog"]')).toBeFalsy() - wrapper.unmount() - vi.useRealTimers() + expect(modalOpen()).toBe(false) }) }) -describe('A server-side upload failure keeps the article unchanged and offers a retry', () => { +describe('A server answer for a file that passed the browser checks', () => { it.each([ - { name: 'a 500 response', trigger: (xhr: FakeXMLHttpRequest) => xhr.resolve(500, '') }, - { name: 'a network failure', trigger: (xhr: FakeXMLHttpRequest) => xhr.fail() }, - ])('shows the failed state for $name', async ({ trigger }) => { - const wrapper = mountUploader() - await openModal(wrapper) - await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) - const xhr = latestXHR() + { + file: 'notes.pdf', + type: 'image/png', + sizeMB: 0.2, + status: 415, + title: '"notes.pdf" can\'t be used', + text: 'Use a PNG, JPEG, WEBP or SVG image, up to 10 MB.', + confirm: 'Choose another file', + }, + { + file: 'north-wall.tif', + type: 'image/png', + sizeMB: 14.2, + status: 413, + title: '"north-wall.tif" can\'t be used', + text: 'north-wall.tif is 14.2 MB. Images can be up to 10 MB.', + confirm: 'Choose another file', + }, + { + file: 'broken.png', + type: 'image/png', + sizeMB: 3, + status: 400, + title: '"broken.png" can\'t be used', + text: 'Use a PNG, JPEG, WEBP or SVG image, up to 10 MB.', + confirm: 'Choose another file', + }, + { + file: 'north-wall.jpg', + type: 'image/jpeg', + sizeMB: 3.9, + status: 500, + title: 'Upload failed', + text: "The server didn't accept the file. Your article hasn't changed.", + confirm: 'Try again', + }, + { + file: 'north-wall.jpg', + type: 'image/jpeg', + sizeMB: 3.9, + status: 429, + title: 'Upload failed', + text: "The server didn't accept the file. Your article hasn't changed.", + confirm: 'Try again', + }, + { + file: 'north-wall.jpg', + type: 'image/jpeg', + sizeMB: 3.9, + status: 0, + title: 'Upload failed', + text: "The server didn't accept the file. Your article hasn't changed.", + confirm: 'Try again', + }, + ])( + 'answers $status with $title', + async ({ file, type, sizeMB, status, title, text, confirm }) => { + await openOnUpload() + const chosen = fileFixture(file, type, sizeMB) + vi.mocked(imageUpload.validateImageFile).mockReturnValueOnce({ ok: true }) + await chooseFile(chosen) + if (findButton('Upload image')) { + await clickButton('Upload image') + } + const xhr = latestXHR() - trigger(xhr) - await flushPromises() + if (status === 0) { + xhr.fail() + } else { + xhr.resolve(status, '') + } + await settle() - expect(document.body.textContent).toContain('Upload failed') - expect(document.body.textContent).toContain( - "The server didn't accept the file. Your article hasn't changed.", - ) - expect(findButton('Try again')).toBeTruthy() - expect(wrapper.emitted('uploaded')).toBeFalsy() - wrapper.unmount() - }) + expect(dialogText()).toContain(title) + expect(dialogText()).toContain(text) + expect(findButton(confirm)).toBeTruthy() + expect(wrapper!.emitted('uploaded')).toBeUndefined() + expect(tabButtons()).toHaveLength(0) + }, + ) }) describe('"Try again" re-attempts the upload with the same file', () => { - it('resends the same file under field "file"', async () => { - const wrapper = mountUploader() - await openModal(wrapper) + it('resends the same file under field "file" without a new lookup', async () => { + await openOnUpload() await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) latestXHR().resolve(500, '') - await flushPromises() + await settle() await clickButton('Try again') expect(FakeXMLHttpRequest.instances).toHaveLength(2) const xhr = latestXHR() - expect(xhr.url).toBe('/v1/article/12/image') + expect(xhr.url).toBe('/v1/media/') expect((xhr.sentBody?.get('file') as File).name).toBe('north-wall.jpg') - expect(document.body.textContent).toContain('north-wall.jpg 0%') - wrapper.unmount() + expect(hashCalls()).toHaveLength(1) + expect(dialogText()).toContain('north-wall.jpg 0%') }) }) -describe('A server 400 for a type the client should have caught shows the same rejected copy', () => { - it('shows the type-rejection copy', async () => { - vi.mocked(imageUpload.validateImageFile).mockReturnValueOnce({ ok: true }) - const wrapper = mountUploader() - await openModal(wrapper) - await chooseRasterFileAndConfirmUpload(fileFixture('notes.pdf', 'image/png', 1.2)) - const xhr = latestXHR() +describe('Session errors are global', () => { + it('hands a 401 on the upload to the login redirect with no local copy', async () => { + await openOnUpload() + await chooseFile(fileFixture('diagram.svg', 'image/svg+xml', 0.04)) - xhr.resolve(400, 'the file must be a PNG, JPEG, WEBP, or SVG image') - await flushPromises() + latestXHR().resolve(401, '') + await settle() - expect(document.body.textContent).toContain(`"notes.pdf" can't be used`) - expect(document.body.textContent).toContain( - 'Use a PNG, JPEG, WEBP or SVG image, up to 10 MB.', - ) - expect(findButton('Choose another file')).toBeTruthy() - expect(wrapper.emitted('uploaded')).toBeFalsy() - wrapper.unmount() + expect(redirectToLogin).toHaveBeenCalledTimes(1) + expect(dialogText()).not.toContain('Upload failed') }) -}) -describe('A server 400 for a size the client should have caught shows the size-named copy', () => { - it('shows the size-rejection copy computed from the file', async () => { - vi.mocked(imageUpload.validateImageFile).mockReturnValueOnce({ ok: true }) - const wrapper = mountUploader() - await openModal(wrapper) - await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.tif', 'image/png', 14.2)) - const xhr = latestXHR() + it('hands a 403 on the upload to the unverified redirect with no local copy', async () => { + await openOnUpload() + await chooseFile(fileFixture('diagram.svg', 'image/svg+xml', 0.04)) - xhr.resolve(400, 'the file exceeds the maximum upload size') - await flushPromises() + latestXHR().resolve(403, '') + await settle() - expect(document.body.textContent).toContain(`"north-wall.tif" can't be used`) - expect(document.body.textContent).toContain( - 'north-wall.tif is 14.2 MB. Images can be up to 10 MB.', - ) - expect(findButton('Choose another file')).toBeTruthy() - expect(wrapper.emitted('uploaded')).toBeFalsy() - wrapper.unmount() + expect(redirectUnverified).toHaveBeenCalledTimes(1) + expect(dialogText()).not.toContain('Upload failed') + }) + + it('stops on a 401 from the hash lookup with no local copy', async () => { + await openOnUpload({ hash: () => Promise.reject(new ApiError(401)) }) + + await chooseFile(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) + + expect(FakeXMLHttpRequest.instances).toHaveLength(0) + expect(dialogText()).not.toContain('Crop and resize') + expect(apiKit.request).toHaveBeenCalled() }) }) diff --git a/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts b/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts index 187fc41..db8bf81 100644 --- a/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts +++ b/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts @@ -1,11 +1,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { defineComponent, nextTick, ref } from 'vue' -import { mount } from '@vue/test-utils' -import uiPlugin from '@nuxt/ui/vue-plugin' +import { nextTick } from 'vue' +import { flushPromises } from '@vue/test-utils' import ImageUploader from '../ImageUploader.vue' +import { installEmptyLibrary, mountUploader, openUploader } from './pickerKit' import { renderCroppedImage } from '@/lib/imageCrop' import type { CropRect } from '@/lib/imageCrop' +vi.mock('@/composables/useApi', async (importOriginal) => { + const actual = await importOriginal<typeof import('@/composables/useApi')>() + return { ...actual, useApi: vi.fn() } +}) + +vi.mock('@/lib/fileHash', () => ({ hashFile: vi.fn(async () => null) })) + vi.mock('@/lib/imageCrop', async (importOriginal) => { const actual = await importOriginal<typeof import('@/lib/imageCrop')>() return { @@ -95,36 +102,7 @@ function fileFixture(name: string, type: string, sizeMB: number) { return file } -const Host = defineComponent({ - components: { ImageUploader }, - props: { - articleId: { type: Number, required: true }, - }, - emits: ['uploaded', 'close'], - setup(_props, { emit }) { - const open = ref(false) - return { - open, - onUploaded: (location: string) => emit('uploaded', location), - onClose: () => emit('close'), - } - }, - template: `<ImageUploader v-model:open="open" :article-id="articleId" @uploaded="onUploaded" @close="onClose" />`, -}) - -function mountUploader() { - return mount(Host, { - props: { articleId: 12 }, - global: { plugins: [uiPlugin] }, - attachTo: document.body, - }) -} - -async function openModal(wrapper: ReturnType<typeof mountUploader>) { - ;(wrapper.vm as unknown as { open: boolean }).open = true - await nextTick() - await nextTick() -} +const openModal = openUploader function fileInput() { return document.body.querySelector<HTMLInputElement>('[data-testid="image-file-input"]')! @@ -138,8 +116,7 @@ async function chooseFile(file: File) { }) input.dispatchEvent(new Event('change')) await nextTick() - await nextTick() - await nextTick() + await flushPromises() } function findButton(text: string) { @@ -151,8 +128,7 @@ function findButton(text: string) { async function clickButton(text: string) { findButton(text)?.click() await nextTick() - await nextTick() - await nextTick() + await flushPromises() } function radioPill(groupLabel: string, text: string) { @@ -187,6 +163,7 @@ function north_wall() { const RealXMLHttpRequest = global.XMLHttpRequest beforeEach(() => { + installEmptyLibrary() FakeXMLHttpRequest.instances = [] global.XMLHttpRequest = FakeXMLHttpRequest as unknown as typeof XMLHttpRequest }) @@ -223,7 +200,7 @@ describe('An SVG skips the crop step and uploads immediately', () => { await chooseFile(fileFixture('diagram.svg', 'image/svg+xml', 0.04)) expect(FakeXMLHttpRequest.instances).toHaveLength(1) - expect(latestXHR().url).toBe('/v1/article/12/image') + expect(latestXHR().url).toBe('/v1/media/') expect(document.body.textContent).not.toContain('Crop and resize') wrapper.unmount() }) diff --git a/web/src/components/__tests__/ImageUploaderLibrary.spec.ts b/web/src/components/__tests__/ImageUploaderLibrary.spec.ts new file mode 100644 index 0000000..c5f5929 --- /dev/null +++ b/web/src/components/__tests__/ImageUploaderLibrary.spec.ts @@ -0,0 +1,522 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { flushPromises } from '@vue/test-utils' +import { ApiError, NetworkError } from '@/composables/useApi' +import { MOBILE_WIDTH } from './viewport' +import { fixture } from '@/views/__tests__/mediaTestKit' +import { + altField, + apiKit, + click, + clickTab, + dialogText, + findButton, + installApi, + listCalls, + modalOpen, + mountUploader, + openUploader, + pageBody, + press, + searchField, + secondPage, + selectedTab, + selectTile, + settle, + skeletonTiles, + textOf, + tile, + tiles, + typeInto, + type PickerRoutes, +} from './pickerKit' + +vi.mock('@/lib/imageCrop', async (importOriginal) => { + const actual = await importOriginal<typeof import('@/lib/imageCrop')>() + return { + ...actual, + readNaturalImageSize: vi.fn(async () => ({ width: 4032, height: 3024 })), + } +}) + +vi.mock('@/composables/useApi', async (importOriginal) => { + const actual = await importOriginal<typeof import('@/composables/useApi')>() + return { ...actual, useApi: vi.fn() } +}) + +function deferred<T>() { + let resolve!: (value: T) => void + const promise = new Promise<T>((res) => { + resolve = res + }) + return { promise, resolve } +} + +let wrapper: ReturnType<typeof mountUploader> | null = null + +afterEach(() => { + wrapper?.unmount() + wrapper = null + vi.useRealTimers() + vi.unstubAllGlobals() + document.body.innerHTML = '' +}) + +async function openLibrary(routes: PickerRoutes = {}) { + installApi(routes) + wrapper = mountUploader() + await openUploader(wrapper) +} + +async function openWithFakeTimers(routes: PickerRoutes = {}, width?: number) { + vi.useFakeTimers() + installApi(routes) + wrapper = mountUploader({}, width) + ;(wrapper.vm as unknown as { open: boolean }).open = true + await vi.advanceTimersByTimeAsync(0) +} + +function summary() { + return textOf(document.body.querySelector('[data-testid="picker-summary"]')) +} + +async function searchFor(value: string) { + await typeInto(searchField(), value) + await new Promise((resolve) => setTimeout(resolve, 150)) + await settle() +} + +describe('The library has images, so Library opens', () => { + it('selects Library with a disabled "Insert image" and draws skeletons only after 150 ms', async () => { + const pending = deferred<unknown>() + await openWithFakeTimers({ list: () => pending.promise }) + + expect(textOf(selectedTab())).toBe('Library') + expect(findButton('Cancel')?.disabled).toBe(false) + expect(findButton('Insert image')?.disabled).toBe(true) + expect(skeletonTiles()).toHaveLength(0) + expect(tiles()).toHaveLength(0) + + await vi.advanceTimersByTimeAsync(150) + + expect(skeletonTiles()).toHaveLength(8) + + pending.resolve(pageBody()) + await vi.advanceTimersByTimeAsync(0) + + expect(skeletonTiles()).toHaveLength(0) + expect(tiles()).toHaveLength(12) + }) + + it('draws 9 skeleton tiles on mobile', async () => { + await openWithFakeTimers({ list: () => new Promise(() => undefined) }, MOBILE_WIDTH) + + await vi.advanceTimersByTimeAsync(150) + + expect(skeletonTiles()).toHaveLength(9) + }) + + it('asks for 24 images and nothing else', async () => { + await openLibrary() + + expect(listCalls()).toHaveLength(1) + expect(listCalls()[0]!.toString()).toBe('limit=24') + }) +}) + +describe('The library is empty, so Upload opens', () => { + it('opens the Upload tab idle, and Library then shows the empty panel', async () => { + await openLibrary({ list: () => pageBody([], null) }) + + expect(textOf(selectedTab())).toBe('Upload') + expect(dialogText()).toContain('Drop an image here, or choose a file') + + await clickTab('Library') + + expect(dialogText()).toContain('Your library is empty') + expect(dialogText()).toContain( + 'Images you upload appear here, ready to use in any article.', + ) + expect(findButton('Insert image')?.disabled).toBe(true) + }) + + it('switches to Upload from "Upload an image"', async () => { + await openLibrary({ list: () => pageBody([], null) }) + await clickTab('Library') + + await click(findButton('Upload an image')) + + expect(textOf(selectedTab())).toBe('Upload') + }) +}) + +describe('A dropped file opens Upload and sends no list request', () => { + it('stays quiet until the Library tab is selected', async () => { + installApi({ hash: () => new Promise(() => undefined) }) + wrapper = mountUploader({ + initialFile: new File(['x'], 'before.png', { type: 'image/png' }), + }) + await openUploader(wrapper) + + expect(textOf(selectedTab())).toBe('Upload') + expect(listCalls()).toHaveLength(0) + + await clickTab('Library') + + expect(listCalls()).toHaveLength(1) + }) +}) + +describe('Library, nothing selected', () => { + it('shows the line, the 12 tiles in order and the tags on the images in the article', async () => { + await openLibrary() + + expect(summary()).toBe('Newest first · 48 images') + expect(tiles().map((element) => element.getAttribute('data-image-id'))).toEqual( + fixture().map((item) => String(item.id)), + ) + const tagged = tiles() + .filter((element) => textOf(element).includes('In this article')) + .map((element) => element.getAttribute('data-image-id')) + expect(tagged).toEqual(['1', '4']) + expect(textOf(tile('north-wall.jpg'))).toContain('north-wall.jpg') + expect(findButton('Load more')?.getAttribute('aria-disabled')).toBeNull() + expect(findButton('Insert image')?.disabled).toBe(true) + }) +}) + +describe('A search is active, so the count is not shown', () => { + it('reads "Newest first"', async () => { + await openLibrary({ + list: (query) => (query.get('q') ? pageBody(fixture([1, 12]), null) : pageBody()), + }) + + await searchFor('wall') + + expect(summary()).toBe('Newest first') + }) +}) + +describe('Load more is hidden on the last page', () => { + it('does not show "Load more" without a cursor', async () => { + await openLibrary({ list: () => pageBody(fixture(), null) }) + + expect(findButton('Load more')).toBeUndefined() + }) +}) + +describe("A tile's accessible name", () => { + it.each([ + ['north-wall.jpg', 'north-wall.jpg, 3.9 MB, used in 2 articles'], + ['kitchen-before.webp', 'kitchen-before.webp, 1.1 MB, used in 1 article'], + ['window-frame.jpg', 'window-frame.jpg, 1.8 MB, unused'], + ])('names %s', async (filename, name) => { + await openLibrary() + + expect(tile(filename).getAttribute('aria-label')).toBe(name) + }) +}) + +describe('A tile is selected by pointer or keyboard', () => { + it.each([ + ['clicks', async () => click(tile('kitchen-after.webp'))], + [ + 'presses Enter', + async () => { + press(tile('kitchen-after.webp'), 'Enter') + await settle() + }, + ], + [ + 'presses Space', + async () => { + press(tile('kitchen-after.webp'), ' ') + await settle() + }, + ], + ])('selects the tile when the user %s on it', async (_name, act) => { + await openLibrary() + + await act() + + expect(tile('kitchen-after.webp').getAttribute('aria-pressed')).toBe('true') + expect( + tile('kitchen-after.webp').querySelector('[data-testid="picker-check"]'), + ).not.toBeNull() + expect(findButton('Insert image')?.disabled).toBe(false) + }) +}) + +describe('The selection panel', () => { + it('shows the thumbnail, the details and the alt text field', async () => { + await openLibrary() + + await selectTile('kitchen-after.webp') + + const panel = textOf(document.body.querySelector('[data-testid="picker-panel"]')) + expect(panel).toContain('kitchen-after.webp') + expect(panel).toContain('2400 × 1600 · 1.2 MB') + expect(panel).toContain('Alt text (optional)') + expect(panel).toContain( + "Describes the image for people who use a screen reader, and shows if it doesn't load. Leave it empty if the image is only decoration.", + ) + expect(altField()?.value).toBe('') + expect(findButton('Load more')).toBeUndefined() + }) + + it('reads "Vector · 8 KB" for an SVG', async () => { + await openLibrary() + + await selectTile('logo-renew.svg') + + expect(textOf(document.body.querySelector('[data-testid="picker-panel"]'))).toContain( + 'Vector · 8 KB', + ) + }) + + it('allows an image already in the article to be selected', async () => { + await openLibrary() + + await selectTile('north-wall.jpg') + + expect(findButton('Insert image')?.disabled).toBe(false) + }) +}) + +describe('Selection is single', () => { + it('selects only the last tile and empties the alt text', async () => { + await openLibrary() + await selectTile('kitchen-after.webp') + await typeInto(altField()!, 'A kitchen') + + await selectTile('window-frame.jpg') + + expect(tile('kitchen-after.webp').getAttribute('aria-pressed')).toBe('false') + expect(tile('window-frame.jpg').getAttribute('aria-pressed')).toBe('true') + expect(altField()!.value).toBe('') + }) + + it('keeps the tile selected, and its alt text, when it is activated again', async () => { + await openLibrary() + await selectTile('kitchen-after.webp') + await typeInto(altField()!, 'A kitchen') + + await selectTile('kitchen-after.webp') + + expect(tile('kitchen-after.webp').getAttribute('aria-pressed')).toBe('true') + expect(altField()!.value).toBe('A kitchen') + }) +}) + +describe('Insert', () => { + function kitchenAfterAtStaticUrl() { + return pageBody( + fixture().map((item) => + item.id === 2 ? { ...item, url: '/static/uploadedImages/2.webp' } : item, + ), + ) + } + + it('emits the item and the alt text, then closes at once with no write request', async () => { + await openLibrary({ list: kitchenAfterAtStaticUrl }) + await selectTile('kitchen-after.webp') + await typeInto(altField()!, 'A kitchen with new oak worktops and white tiles') + + await click(findButton('Insert image')) + + const [item, alt] = wrapper!.emitted('inserted')![0] as [ + { id: number; url: string }, + string, + ] + expect(item.id).toBe(2) + expect(item.url).toBe('/static/uploadedImages/2.webp') + expect(alt).toBe('A kitchen with new oak worktops and white tiles') + expect(modalOpen()).toBe(false) + expect(apiKit.request.mock.calls.every(([, options]) => !options?.method)).toBe(true) + }) + + it('inserts an empty alt for a decorative image', async () => { + await openLibrary() + await selectTile('kitchen-after.webp') + + await click(findButton('Insert image')) + + expect(wrapper!.emitted('inserted')![0]![1]).toBe('') + }) + + it.each([ + ['activates "Cancel"', async () => click(findButton('Cancel'))], + [ + 'activates the x', + async () => + click(document.body.querySelector('[role="dialog"] button[aria-label="Close"]')), + ], + [ + 'presses Escape', + async () => { + document.body + .querySelector('[role="dialog"]')! + .dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })) + await settle() + }, + ], + ])('changes nothing when the user %s', async (_name, act) => { + await openLibrary() + await selectTile('kitchen-after.webp') + await typeInto(altField()!, 'A kitchen') + + await act() + + expect(modalOpen()).toBe(false) + expect(wrapper!.emitted('inserted')).toBeUndefined() + expect(wrapper!.emitted('uploaded')).toBeUndefined() + }) +}) + +describe('Search', () => { + it('searches after a 120 ms debounce, with no cursor, and clears the selection', async () => { + await openWithFakeTimers() + tile('kitchen-after.webp').click() + await vi.advanceTimersByTimeAsync(0) + expect(findButton('Insert image')?.disabled).toBe(false) + + await typeInto(searchField(), 'wall') + await vi.advanceTimersByTimeAsync(119) + + expect(listCalls()).toHaveLength(1) + + await vi.advanceTimersByTimeAsync(1) + + expect(listCalls()).toHaveLength(2) + expect(listCalls()[1]!.get('q')).toBe('wall') + expect(listCalls()[1]!.get('limit')).toBe('24') + expect(listCalls()[1]!.has('cursor')).toBe(false) + expect(findButton('Insert image')?.disabled).toBe(true) + }) + + it('draws skeleton tiles while a search loads, after 150 ms', async () => { + await openWithFakeTimers({ + list: (query) => (query.get('q') ? new Promise(() => undefined) : pageBody()), + }) + + await typeInto(searchField(), 'wall') + await vi.advanceTimersByTimeAsync(120) + + expect(skeletonTiles()).toHaveLength(0) + + await vi.advanceTimersByTimeAsync(150) + + expect(skeletonTiles()).toHaveLength(8) + }) + + it('clears the search from the x in the field', async () => { + await openLibrary({ + list: (query) => (query.get('q') ? pageBody(fixture([1, 12]), null) : pageBody()), + }) + await searchFor('wall') + + await click(document.body.querySelector('button[aria-label="Clear search"]')) + + expect(searchField().value).toBe('') + const last = listCalls().at(-1)! + expect(last.has('q')).toBe(false) + expect(last.get('limit')).toBe('24') + }) + + it('shows the no results panel and "Clear search" empties the field', async () => { + await openLibrary({ + list: (query) => (query.get('q') ? pageBody([], null) : pageBody()), + }) + await searchFor('wall') + + expect(dialogText()).toContain('No images match "wall"') + expect(dialogText()).toContain( + 'Check the spelling, or search for a shorter part of the filename.', + ) + expect(searchField().value).toBe('wall') + expect(findButton('Insert image')?.disabled).toBe(true) + + await click(findButton('Clear search')) + + expect(searchField().value).toBe('') + expect(listCalls().at(-1)!.has('q')).toBe(false) + expect(tiles()).toHaveLength(12) + }) +}) + +describe('Load more', () => { + it('shows progress, appends the new tiles and moves focus to the first one', async () => { + const more = deferred<unknown>() + await openWithFakeTimers({ + list: (query) => (query.get('cursor') ? more.promise : pageBody()), + }) + const button = findButton('Load more')! + button.focus() + + button.click() + await vi.advanceTimersByTimeAsync(0) + + const last = listCalls().at(-1)! + expect(last.get('cursor')).toBe('c24') + expect(last.get('limit')).toBe('24') + const loading = findButton('Loading…')! + expect(loading.getAttribute('aria-disabled')).toBe('true') + expect(document.activeElement).toBe(loading) + await vi.advanceTimersByTimeAsync(150) + expect(skeletonTiles()).toHaveLength(4) + + more.resolve(pageBody(secondPage(), null)) + await vi.advanceTimersByTimeAsync(0) + await flushPromises() + + expect(tiles()).toHaveLength(24) + expect(skeletonTiles()).toHaveLength(0) + expect(document.activeElement).toBe(tiles()[12]) + }) + + it('draws a row of 3 skeleton tiles on mobile', async () => { + await openWithFakeTimers( + { list: (query) => (query.get('cursor') ? new Promise(() => undefined) : pageBody()) }, + MOBILE_WIDTH, + ) + + findButton('Load more')!.click() + await vi.advanceTimersByTimeAsync(150) + + expect(skeletonTiles()).toHaveLength(3) + }) + + it('combines the cursor with the search', async () => { + await openLibrary({ + list: (query) => pageBody(query.get('q') ? fixture([1, 2]) : fixture(), 'c24'), + }) + await searchFor('wall') + + await click(findButton('Load more')) + + const last = listCalls().at(-1)! + expect(last.get('q')).toBe('wall') + expect(last.get('limit')).toBe('24') + expect(last.get('cursor')).toBe('c24') + }) +}) + +describe('The library fails to load', () => { + it.each([ + ['a 500 response', () => Promise.reject(new ApiError(500))], + ['a request that never reaches the server', () => Promise.reject(new NetworkError())], + ])('shows the alert for %s and retries', async (_name, failure) => { + let failing = true + await openLibrary({ list: () => (failing ? failure() : pageBody()) }) + + expect(dialogText()).toContain("Couldn't load your library") + expect(dialogText()).toContain('Check your connection and try again.') + expect(findButton('Insert image')?.disabled).toBe(true) + + failing = false + await click(findButton('Try again')) + + expect(listCalls()).toHaveLength(2) + expect(tiles()).toHaveLength(12) + expect(dialogText()).not.toContain("Couldn't load your library") + }) +}) diff --git a/web/src/components/__tests__/pickerKit.ts b/web/src/components/__tests__/pickerKit.ts new file mode 100644 index 0000000..d69b0af --- /dev/null +++ b/web/src/components/__tests__/pickerKit.ts @@ -0,0 +1,190 @@ +import { vi } from 'vitest' +import { defineComponent, nextTick, ref } from 'vue' +import { flushPromises, mount } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import uiPlugin from '@nuxt/ui/vue-plugin' +import ImageUploader from '../ImageUploader.vue' +import { useApi } from '@/composables/useApi' +import type { MediaItem } from '@/composables/useMedia' +import { DESKTOP_WIDTH, setViewportWidth } from './viewport' +import { fixture, textOf } from '@/views/__tests__/mediaTestKit' + +export { textOf } + +export const FIRST_PAGE_CURSOR = 'c24' +export const PICKER_COUNTS = { all: 48, unused: 2, duplicates: 2 } + +export function pageBody( + items: MediaItem[] = fixture(), + next_cursor: string | null = FIRST_PAGE_CURSOR, +) { + return { items, next_cursor, counts: PICKER_COUNTS } +} + +export function secondPage() { + return fixture().map((item) => ({ + ...item, + id: item.id + 12, + filename: `more-${item.filename}`, + url: `/media/more-${item.filename}`, + })) +} + +export interface PickerRoutes { + list?: (query: URLSearchParams) => unknown + hash?: (hash: string) => unknown +} + +export const apiKit = { request: vi.fn() } + +export function installApi(routes: PickerRoutes = {}) { + apiKit.request = vi.fn(async (path: string) => { + const url = new URL(path, 'http://localhost') + const hash = url.searchParams.get('hash') + if (hash !== null) { + return (routes.hash ?? (() => pageBody([], null)))(hash) + } + return (routes.list ?? (() => pageBody()))(url.searchParams) + }) + vi.mocked(useApi).mockReturnValue({ + getMe: vi.fn().mockResolvedValue({ verified: true, username: 'marta', id: 1 }), + request: apiKit.request, + }) +} + +export function installEmptyLibrary(routes: PickerRoutes = {}) { + installApi({ list: () => pageBody([], null), ...routes }) +} + +export function listCalls() { + return apiKit.request.mock.calls + .map(([path]) => new URL(path as string, 'http://localhost')) + .filter((url) => url.pathname === '/v1/media/' && !url.searchParams.has('hash')) + .map((url) => url.searchParams) +} + +export function hashCalls() { + return apiKit.request.mock.calls + .map(([path]) => new URL(path as string, 'http://localhost')) + .filter((url) => url.searchParams.has('hash')) + .map((url) => url.searchParams.get('hash')) +} + +export const Host = defineComponent({ + components: { ImageUploader }, + props: { + initialFile: { type: Object as () => File | null, default: null }, + inArticleIds: { type: Array as () => number[], default: () => [1, 4] }, + }, + emits: ['uploaded', 'inserted', 'close'], + setup(_props, { emit }) { + const open = ref(false) + return { + open, + onUploaded: (item: MediaItem) => emit('uploaded', item), + onInserted: (item: MediaItem, alt: string) => emit('inserted', item, alt), + onClose: () => emit('close'), + } + }, + template: `<ImageUploader v-model:open="open" :initial-file="initialFile" :in-article-ids="inArticleIds" @uploaded="onUploaded" @inserted="onInserted" @close="onClose" />`, +}) + +export function mountUploader( + props: { initialFile?: File | null; inArticleIds?: number[] } = {}, + width = DESKTOP_WIDTH, +) { + setViewportWidth(width) + setActivePinia(createPinia()) + return mount(Host, { + props, + global: { plugins: [uiPlugin] }, + attachTo: document.body, + }) +} + +export async function openUploader(wrapper: ReturnType<typeof mountUploader>) { + ;(wrapper.vm as unknown as { open: boolean }).open = true + await nextTick() + await nextTick() + await flushPromises() +} + +export async function settle() { + await flushPromises() + await nextTick() + await flushPromises() +} + +export function tabButtons() { + return Array.from(document.body.querySelectorAll<HTMLElement>('[role="tab"]')) +} + +export function tabButton(label: string) { + return tabButtons().find((element) => textOf(element) === label) +} + +export function selectedTab() { + return tabButtons().find((element) => element.getAttribute('aria-selected') === 'true') +} + +export function tiles() { + return Array.from(document.body.querySelectorAll<HTMLButtonElement>('[data-picker-tile]')) +} + +export function tile(filename: string) { + return tiles().find((element) => + element.getAttribute('aria-label')?.startsWith(`${filename},`), + )! +} + +export function skeletonTiles() { + return document.body.querySelectorAll('[data-testid="picker-skeleton-tile"]') +} + +export function findButton(label: string) { + return Array.from(document.body.querySelectorAll<HTMLButtonElement>('button')).find( + (button) => textOf(button) === label, + ) +} + +export async function click(element: Element | null | undefined) { + ;(element as HTMLElement).click() + await settle() +} + +export async function clickTab(label: string) { + tabButton(label)!.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0 })) + await settle() +} + +export async function selectTile(filename: string) { + await click(tile(filename)) +} + +export function altField() { + return document.body.querySelector<HTMLInputElement>( + 'input[placeholder="e.g. North wall of the kitchen before plastering"]', + ) +} + +export async function typeInto(input: HTMLInputElement, value: string) { + input.value = value + input.dispatchEvent(new Event('input', { bubbles: true })) + await nextTick() +} + +export function searchField() { + return document.body.querySelector<HTMLInputElement>('input[aria-label="Search by filename"]')! +} + +export function dialogText() { + return textOf(document.body.querySelector('[role="dialog"]')) +} + +export function modalOpen() { + return document.body.querySelector('[role="dialog"]') !== null +} + +export function press(element: Element, key: string) { + element.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true })) +} diff --git a/web/src/composables/useArticles.ts b/web/src/composables/useArticles.ts index 0e5e813..906ba3a 100644 --- a/web/src/composables/useArticles.ts +++ b/web/src/composables/useArticles.ts @@ -1,15 +1,9 @@ import { useApi } from '@/composables/useApi' -export interface ArticleImage { - id: number - url: string -} - export interface Article { id: number title: string body: string - images: ArticleImage[] is_online: boolean created_at: string updated_at: string diff --git a/web/src/composables/useImageUpload.ts b/web/src/composables/useImageUpload.ts index 5c94e99..3fca5e8 100644 --- a/web/src/composables/useImageUpload.ts +++ b/web/src/composables/useImageUpload.ts @@ -1,4 +1,14 @@ import { ref } from 'vue' +import { redirectToLogin, redirectUnverified } from '@/composables/useApi' +import type { MediaItem } from '@/composables/useMedia' + +const UPLOAD_URL = '/v1/media/' +const HTTP_CREATED = 201 +const HTTP_BAD_REQUEST = 400 +const HTTP_UNAUTHORIZED = 401 +const HTTP_FORBIDDEN = 403 +const HTTP_TOO_LARGE = 413 +const HTTP_UNSUPPORTED = 415 export interface UploadProgress { loaded: number @@ -6,10 +16,41 @@ export interface UploadProgress { } export type UploadOutcome = - | { kind: 'success'; location: string; id: number } - | { kind: 'rejected'; body: string } + | { kind: 'success'; item: MediaItem } + | { kind: 'rejected'; reason: 'size' | 'type' } + | { kind: 'handled' } | { kind: 'failed' } +function readItem(responseText: string): MediaItem | null { + try { + return JSON.parse(responseText) as MediaItem + } catch { + return null + } +} + +function outcomeOf(request: XMLHttpRequest): UploadOutcome { + switch (request.status) { + case HTTP_CREATED: { + const item = readItem(request.responseText) + return item ? { kind: 'success', item } : { kind: 'failed' } + } + case HTTP_TOO_LARGE: + return { kind: 'rejected', reason: 'size' } + case HTTP_UNSUPPORTED: + case HTTP_BAD_REQUEST: + return { kind: 'rejected', reason: 'type' } + case HTTP_UNAUTHORIZED: + redirectToLogin() + return { kind: 'handled' } + case HTTP_FORBIDDEN: + redirectUnverified() + return { kind: 'handled' } + default: + return { kind: 'failed' } + } +} + export function useImageUpload() { const progress = ref<UploadProgress>({ loaded: 0, total: 0 }) let activeRequest: XMLHttpRequest | null = null @@ -19,31 +60,19 @@ export function useImageUpload() { activeRequest = null } - function start(articleId: number, file: File): Promise<UploadOutcome> { + function start(file: File): Promise<UploadOutcome> { progress.value = { loaded: 0, total: file.size } return new Promise((resolve) => { const request = new XMLHttpRequest() activeRequest = request - request.open('POST', `/v1/article/${articleId}/image`) + request.open('POST', UPLOAD_URL) request.withCredentials = true request.upload.addEventListener('progress', (event) => { progress.value = { loaded: event.loaded, total: event.total } }) request.addEventListener('load', () => { activeRequest = null - if (request.status === 201) { - const body = JSON.parse(request.responseText) as { - id: number - location: string - } - resolve({ kind: 'success', location: body.location, id: body.id }) - return - } - if (request.status === 400) { - resolve({ kind: 'rejected', body: request.responseText }) - return - } - resolve({ kind: 'failed' }) + resolve(outcomeOf(request)) }) request.addEventListener('error', () => { activeRequest = null diff --git a/web/src/composables/useLibraryList.ts b/web/src/composables/useLibraryList.ts new file mode 100644 index 0000000..dddfb90 --- /dev/null +++ b/web/src/composables/useLibraryList.ts @@ -0,0 +1,171 @@ +import { ref } from 'vue' +import { tryOnBeforeUnmount } from '@vueuse/core' +import { useApi } from '@/composables/useApi' +import { + MEDIA_SKELETON_DELAY_MS, + classifyMediaError, + type MediaCounts, + type MediaItem, +} from '@/composables/useMedia' +import { useSeenImagesStore } from '@/stores/seenImages' + +export const LIBRARY_PAGE_LIMIT = 24 + +export type LibraryStatus = 'idle' | 'loading' | 'ready' | 'error' | 'handled' + +interface LibraryResponse { + items: MediaItem[] + next_cursor: string | null + counts: MediaCounts +} + +function requestQuery(query: string, cursor: string | null) { + return [ + ...(query ? [`q=${encodeURIComponent(query)}`] : []), + `limit=${LIBRARY_PAGE_LIMIT}`, + ...(cursor ? [`cursor=${encodeURIComponent(cursor)}`] : []), + ].join('&') +} + +export function useLibraryList() { + const { request } = useApi() + const seen = useSeenImagesStore() + + const items = ref<MediaItem[]>([]) + const total = ref(0) + const nextCursor = ref<string | null>(null) + const status = ref<LibraryStatus>('idle') + const query = ref('') + const showSkeleton = ref(false) + const loadingMore = ref(false) + const firstNewId = ref<number | null>(null) + let skeletonTimer: ReturnType<typeof setTimeout> | null = null + let latest = 0 + + function clearSkeletonTimer() { + if (skeletonTimer !== null) { + clearTimeout(skeletonTimer) + skeletonTimer = null + } + } + + function startSkeletonTimer() { + showSkeleton.value = false + clearSkeletonTimer() + skeletonTimer = setTimeout(() => { + showSkeleton.value = true + }, MEDIA_SKELETON_DELAY_MS) + } + + function stopSkeleton() { + clearSkeletonTimer() + showSkeleton.value = false + } + + function reset() { + latest += 1 + items.value = [] + total.value = 0 + nextCursor.value = null + status.value = 'idle' + query.value = '' + loadingMore.value = false + firstNewId.value = null + stopSkeleton() + } + + async function load(nextQuery = query.value) { + latest += 1 + const ticket = latest + query.value = nextQuery + items.value = [] + nextCursor.value = null + loadingMore.value = false + firstNewId.value = null + status.value = 'loading' + startSkeletonTimer() + try { + const response = await request<LibraryResponse>( + `/v1/media/?${requestQuery(nextQuery, null)}`, + ) + if (ticket !== latest) { + return + } + items.value = response.items + total.value = response.counts.all + nextCursor.value = response.next_cursor + seen.remember(response.items) + status.value = 'ready' + } catch (error) { + if (ticket !== latest) { + return + } + status.value = classifyMediaError(error) === 'handled' ? 'handled' : 'error' + } finally { + if (ticket === latest) { + stopSkeleton() + } + } + } + + async function loadMore() { + if (nextCursor.value === null || loadingMore.value || status.value !== 'ready') { + return + } + const ticket = latest + loadingMore.value = true + startSkeletonTimer() + try { + const response = await request<LibraryResponse>( + `/v1/media/?${requestQuery(query.value, nextCursor.value)}`, + ) + if (ticket !== latest) { + return + } + const known = new Set(items.value.map((item) => item.id)) + const added = response.items.filter((item) => !known.has(item.id)) + items.value = [...items.value, ...added] + nextCursor.value = response.next_cursor + firstNewId.value = added[0]?.id ?? null + seen.remember(added) + } catch { + return + } finally { + if (ticket === latest) { + loadingMore.value = false + stopSkeleton() + } + } + } + + async function search(nextQuery: string) { + if (nextQuery === query.value && status.value !== 'idle') { + return + } + await load(nextQuery) + } + + async function ensureLoaded() { + if (status.value === 'idle') { + await load() + } + } + + tryOnBeforeUnmount(clearSkeletonTimer) + + return { + items, + total, + nextCursor, + status, + query, + showSkeleton, + loadingMore, + firstNewId, + load, + loadMore, + search, + ensureLoaded, + reset, + } +} diff --git a/web/src/composables/useMediaUpload.ts b/web/src/composables/useMediaUpload.ts index f15c334..71d76fd 100644 --- a/web/src/composables/useMediaUpload.ts +++ b/web/src/composables/useMediaUpload.ts @@ -41,7 +41,7 @@ function toneOf(name: string) { return Array.from(name).reduce((sum, letter) => sum + letter.charCodeAt(0), 0) % TONE_COUNT } -function oldestOf(items: MediaItem[]) { +export function oldestOf(items: MediaItem[]) { return [...items].sort( (first, second) => Date.parse(first.created_at) - Date.parse(second.created_at) || first.id - second.id, diff --git a/web/src/lib/__tests__/articleImage.spec.ts b/web/src/lib/__tests__/articleImage.spec.ts index ef4035f..6a6215b 100644 --- a/web/src/lib/__tests__/articleImage.spec.ts +++ b/web/src/lib/__tests__/articleImage.spec.ts @@ -61,6 +61,23 @@ describe('Selecting an inserted image reveals its delete button', () => { }) }) +describe('The remove button is labelled and neutral', () => { + it('reads "Remove image" with a trash icon and is not styled as destructive', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + editor.commands.setNodeSelection(findArticleImagePos(editor, 10) as number) + await flush() + + const button = deleteButton()! + + expect(button.textContent?.trim()).toBe('Remove image') + expect(button.className).not.toContain('text-error') + expect(button.className).toContain('border-accented') + }) +}) + describe('An image with no data-image-id gets no delete affordance', () => { it('renders no delete button even when the node is selected', async () => { const { wrapper, getEditor } = mountHarness() diff --git a/web/src/lib/__tests__/bodyImageIds.spec.ts b/web/src/lib/__tests__/bodyImageIds.spec.ts new file mode 100644 index 0000000..99c9689 --- /dev/null +++ b/web/src/lib/__tests__/bodyImageIds.spec.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest' +import { bodyImageIds } from '../bodyImageIds' + +describe('bodyImageIds', () => { + it('lists the ids of the images that carry data-image-id', () => { + const body = + '<p>a</p><img src="/a.jpg" alt="" data-image-id="1"><p>b</p><img data-image-id="4" src="/b.svg">' + + expect(bodyImageIds(body)).toEqual([1, 4]) + }) + + it('ignores images without an id and non numeric ids', () => { + const body = + '<img src="/a.jpg"><img src="/b.jpg" data-image-id="abc"><img data-image-id="">' + + expect(bodyImageIds(body)).toEqual([]) + }) + + it('returns nothing for a body without images', () => { + expect(bodyImageIds('<p>Hello</p>')).toEqual([]) + }) +}) diff --git a/web/src/lib/bodyImageIds.ts b/web/src/lib/bodyImageIds.ts new file mode 100644 index 0000000..436d33f --- /dev/null +++ b/web/src/lib/bodyImageIds.ts @@ -0,0 +1,13 @@ +const IMAGE_ID_SELECTOR = 'img[data-image-id]' +const IMAGE_ID_ATTRIBUTE = 'data-image-id' + +export function bodyImageCount(html: string): number { + return new DOMParser().parseFromString(html, 'text/html').querySelectorAll('img').length +} + +export function bodyImageIds(html: string): number[] { + const document = new DOMParser().parseFromString(html, 'text/html') + return Array.from(document.querySelectorAll(IMAGE_ID_SELECTOR)) + .map((image) => Number.parseInt(image.getAttribute(IMAGE_ID_ATTRIBUTE) ?? '', 10)) + .filter((id) => !Number.isNaN(id)) +} diff --git a/web/src/lib/mediaFormat.ts b/web/src/lib/mediaFormat.ts index 7471269..041b91f 100644 --- a/web/src/lib/mediaFormat.ts +++ b/web/src/lib/mediaFormat.ts @@ -76,3 +76,11 @@ export function joinNames(names: string[]) { } return `${names.slice(0, -1).join(', ')} and ${names[names.length - 1]}` } + +export function libraryMatchText( + filename: string, + existing: { created_at: string; uploaded_by: string }, +) { + const by = existing.uploaded_by ? ` by ${existing.uploaded_by}` : '' + return `${filename} matches an image uploaded on ${formatMediaDate(existing.created_at)}${by}.` +} diff --git a/web/src/lib/tabStyles.ts b/web/src/lib/tabStyles.ts new file mode 100644 index 0000000..f9748f4 --- /dev/null +++ b/web/src/lib/tabStyles.ts @@ -0,0 +1,2 @@ +export const UNDERLINE_TAB_TRIGGER = + 'h-9 px-0 py-0 data-[state=inactive]:text-toned in-[[data-slot=list]:not(:has([data-slot=indicator]))]:data-[state=active]:after:bottom-0 in-[[data-slot=list]:not(:has([data-slot=indicator]))]:data-[state=active]:after:h-0.5 max-md:h-11 max-md:text-[0.9375rem]' diff --git a/web/src/stores/__tests__/seenImages.spec.ts b/web/src/stores/__tests__/seenImages.spec.ts new file mode 100644 index 0000000..db412a2 --- /dev/null +++ b/web/src/stores/__tests__/seenImages.spec.ts @@ -0,0 +1,30 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { useSeenImagesStore } from '../seenImages' + +beforeEach(() => { + setActivePinia(createPinia()) +}) + +describe('seenImages store', () => { + it('finds a remembered image by id', () => { + const store = useSeenImagesStore() + + store.remember([{ id: 1, filename: 'north-wall.jpg', size: 4089446, extra: true } as never]) + + expect(store.find(1)).toEqual({ id: 1, filename: 'north-wall.jpg', size: 4089446 }) + }) + + it('knows nothing about an image it has not seen', () => { + expect(useSeenImagesStore().find(7)).toBeNull() + }) + + it('keeps the latest record of an image', () => { + const store = useSeenImagesStore() + store.remember([{ id: 1, filename: 'north-wall.jpg', size: 10 }]) + + store.remember([{ id: 1, filename: 'renamed.jpg', size: 10 }]) + + expect(store.find(1)?.filename).toBe('renamed.jpg') + }) +}) diff --git a/web/src/stores/seenImages.ts b/web/src/stores/seenImages.ts new file mode 100644 index 0000000..be2e76f --- /dev/null +++ b/web/src/stores/seenImages.ts @@ -0,0 +1,24 @@ +import { defineStore } from 'pinia' +import { ref } from 'vue' + +export interface SeenImage { + id: number + filename: string + size: number +} + +export const useSeenImagesStore = defineStore('seenImages', () => { + const byId = ref<Record<number, SeenImage>>({}) + + function remember(images: SeenImage[]) { + for (const { id, filename, size } of images) { + byId.value[id] = { id, filename, size } + } + } + + function find(id: number): SeenImage | null { + return byId.value[id] ?? null + } + + return { remember, find } +}) diff --git a/web/src/views/ArticleView.vue b/web/src/views/ArticleView.vue index 961d46d..17d9dfe 100644 --- a/web/src/views/ArticleView.vue +++ b/web/src/views/ArticleView.vue @@ -10,6 +10,7 @@ import NewArticleModal from '@/components/NewArticleModal.vue' import ConfirmDialog from '@/components/ConfirmDialog.vue' import { ApiError, useApi } from '@/composables/useApi' import { useArticles, type Article } from '@/composables/useArticles' +import { bodyImageCount } from '@/lib/bodyImageIds' const SKELETON_DELAY_MS = 150 const SEARCH_DEBOUNCE_MS = 120 @@ -239,7 +240,7 @@ async function handleCopyUrl(id: number) { } function deleteDescription(article: Article) { - const count = article.images.length + const count = bodyImageCount(article.body) const imagesPhrase = count === 0 ? '' : count === 1 ? ' and its image' : ` and its ${count} images` return `"${article.title}"${imagesPhrase} will be deleted. This can't be undone.` diff --git a/web/src/views/EditorView.vue b/web/src/views/EditorView.vue index 1bdb90e..ef0265a 100644 --- a/web/src/views/EditorView.vue +++ b/web/src/views/EditorView.vue @@ -15,6 +15,10 @@ import ImageUploader from '@/components/ImageUploader.vue' import { ApiError, authRedirectPending, useApi } from '@/composables/useApi' import type { Article } from '@/composables/useArticles' import { useArticleDraft } from '@/composables/useArticleDraft' +import type { MediaItem } from '@/composables/useMedia' +import { bodyImageCount, bodyImageIds } from '@/lib/bodyImageIds' +import { formatSize } from '@/lib/mediaFormat' +import { useSeenImagesStore } from '@/stores/seenImages' const SKELETON_DELAY_MS = 150 const BODY_MAX_LENGTH = 10000 @@ -29,6 +33,7 @@ const toast = useToast() const articleId = Number(route.params.id) const draft = useArticleDraft(articleId) +const seenImages = useSeenImagesStore() const loadState = ref<LoadState>('loading') const showSkeleton = ref(false) @@ -39,6 +44,7 @@ const publishPending = ref<string | undefined>(undefined) const editorRef = ref<InstanceType<typeof ArticleEditor> | null>(null) const imageUploaderOpen = ref(false) const imageUploaderFile = ref<File | null>(null) +const imageIdsInArticle = ref<number[]>([]) let skeletonTimer: ReturnType<typeof setTimeout> | null = null function clearSkeletonTimer() { @@ -159,7 +165,7 @@ function handleUnpublish() { } function deleteDescription() { - const count = article.value?.images.length ?? 0 + const count = bodyImageCount(article.value?.body ?? '') const imagesPhrase = count === 0 ? '' : count === 1 ? ' and its image' : ` and its ${count} images` return `"${article.value?.title ?? ''}"${imagesPhrase} will be deleted. This can't be undone.` @@ -206,6 +212,7 @@ async function handleDelete() { function openImageUploader(file: File | null) { imageUploaderFile.value = file + imageIdsInArticle.value = bodyImageIds(draft.body.value) imageUploaderOpen.value = true } @@ -221,8 +228,7 @@ function onEditorDrop(event: DragEvent) { openImageUploader(file) } -function onImageUploaded(location: string, imageId: number) { - editorRef.value?.insertImage(location, '', imageId) +function announceImageAdded() { toast.add({ color: 'success', icon: 'i-lucide-circle-check', @@ -231,31 +237,49 @@ function onImageUploaded(location: string, imageId: number) { }) } +function onImageUploaded(item: MediaItem) { + seenImages.remember([item]) + editorRef.value?.insertImage(item.url, '', item.id) + announceImageAdded() +} + +function onImageInserted(item: MediaItem, alt: string) { + seenImages.remember([item]) + editorRef.value?.insertImage(item.url, alt, item.id) + announceImageAdded() +} + async function performRemoveImage(imageId: number) { - try { - await request(`/v1/article/${articleId}/image/${imageId}`, { method: 'DELETE' }) - } catch { - throw new Error("Couldn't remove the image, so it's still there. Try again in a moment.") - } editorRef.value?.removeImage(imageId) toast.add({ color: 'success', icon: 'i-lucide-circle-check', title: 'Image removed', - description: 'Save to keep this change.', + description: "It's still in your library.", }) } +function removalPreview(imageId: number) { + const seen = seenImages.find(imageId) + return { + src: editorRef.value?.imageSrc(imageId) ?? '', + filename: seen?.filename, + size: seen ? formatSize(seen.size) : undefined, + } +} + async function onRequestRemoveImage(imageId: number) { await overlay .create(ConfirmDialog, { props: { - title: 'Remove this image?', - description: "This image will be removed from the article. This can't be undone.", + title: 'Remove this image from the article?', + description: + "It's removed from this article only. The file stays in your library and in any other article that uses it.", confirmLabel: 'Remove image', cancelLabel: 'Cancel', pendingLabel: 'Removing…', - tone: 'danger', + tone: 'neutral', + preview: removalPreview(imageId), action: () => performRemoveImage(imageId), }, }) @@ -476,9 +500,10 @@ onBeforeRouteLeave(async () => { <ImageUploader v-model:open="imageUploaderOpen" - :article-id="articleId" :initial-file="imageUploaderFile" + :in-article-ids="imageIdsInArticle" @uploaded="onImageUploaded" + @inserted="onImageInserted" @close="onImageUploaderClosed" /> diff --git a/web/src/views/__tests__/ArticleView.spec.ts b/web/src/views/__tests__/ArticleView.spec.ts index c9163c7..6bb6e72 100644 --- a/web/src/views/__tests__/ArticleView.spec.ts +++ b/web/src/views/__tests__/ArticleView.spec.ts @@ -77,7 +77,6 @@ function sixArticleFixture(): Article[] { id: 1, title: 'How we price a bathroom renovation', body: '', - images: [], is_online: true, created_at: '2026-03-12T09:14:00Z', updated_at: iso(2 * HOUR_MS), @@ -86,7 +85,6 @@ function sixArticleFixture(): Article[] { id: 2, title: 'Choosing lime plaster for old stone walls', body: '', - images: [], is_online: false, created_at: '2026-09-09T09:00:00Z', updated_at: iso(1 * DAY_MS), @@ -95,7 +93,6 @@ function sixArticleFixture(): Article[] { id: 3, title: 'Before and after: the Rue Verte kitchen', body: '', - images: [], is_online: true, created_at: '2026-08-28T09:00:00Z', updated_at: iso(3 * DAY_MS), @@ -103,12 +100,7 @@ function sixArticleFixture(): Article[] { { id: 4, title: 'Winter checklist for timber windows', - body: '', - images: [ - { id: 10, url: '/i/10.png' }, - { id: 11, url: '/i/11.png' }, - { id: 12, url: '/i/12.png' }, - ], + body: '<img src="/i/10.png"><img src="/i/11.png"><img src="/i/12.png">', is_online: false, created_at: '2026-09-02T09:00:00Z', updated_at: iso(5 * DAY_MS), @@ -117,7 +109,6 @@ function sixArticleFixture(): Article[] { id: 5, title: 'Why we reuse original floorboards', body: '', - images: [], is_online: true, created_at: '2026-06-14T09:00:00Z', updated_at: '2026-08-18T09:00:00Z', @@ -126,7 +117,6 @@ function sixArticleFixture(): Article[] { id: 6, title: 'Our first passive-house retrofit', body: '', - images: [], is_online: true, created_at: '2026-04-03T09:00:00Z', updated_at: '2026-07-02T09:00:00Z', diff --git a/web/src/views/__tests__/EditorImageUpload.spec.ts b/web/src/views/__tests__/EditorImageUpload.spec.ts index 9959d93..266f185 100644 --- a/web/src/views/__tests__/EditorImageUpload.spec.ts +++ b/web/src/views/__tests__/EditorImageUpload.spec.ts @@ -10,6 +10,8 @@ import RootComponent from '@/App.vue' const FIXED_NOW = '2026-09-26T11:02:00Z' +vi.mock('@/lib/fileHash', () => ({ hashFile: vi.fn(async () => 'a'.repeat(64)) })) + vi.mock('@/lib/imageCrop', async (importOriginal) => { const actual = await importOriginal<typeof import('@/lib/imageCrop')>() return { @@ -118,7 +120,6 @@ function articleFixture(overrides: Partial<Article> = {}): Article { id: 12, title: 'How we price a bathroom renovation', body: '<p>a</p>', - images: [], is_online: false, created_at: '2026-03-12T09:14:00Z', updated_at: FIXED_NOW, @@ -186,29 +187,39 @@ afterEach(async () => { await router.push('/login') }) -describe("Opening the uploader from the toolbar's image button shows the idle state", () => { - it('opens ImageUploader idle and sends no request', async () => { - const wrapper = await mountEditor({ - 'GET /v1/article/12': () => mockResponse(200, articleFixture()), - }) +const EMPTY_LIBRARY = { items: [], next_cursor: null, counts: { all: 0, unused: 0, duplicates: 0 } } + +function articleHandlers(extra: Record<string, () => Response | Promise<Response>> = {}) { + return { + 'GET /v1/article/12': () => mockResponse(200, articleFixture()), + 'GET /v1/media/?limit=24': () => mockResponse(200, EMPTY_LIBRARY), + ...extra, + } +} + +describe("Opening the uploader from the toolbar's image button on an empty library shows Upload", () => { + it('opens ImageUploader on the idle Upload tab and sends no POST', async () => { + const wrapper = await mountEditor(articleHandlers()) await toolbarImageButton(wrapper)?.trigger('click') await flushPromises() expect(document.body.textContent).toContain('Add an image') - expect(document.body.textContent).toContain('Drop an image here') + expect(document.body.textContent).toContain('Drop an image here, or choose a file') expect(document.body.textContent).toContain('PNG, JPEG, WEBP, SVG · max 10 MB') - expect(findButton('Choose file')).toBeTruthy() expect(findButton('Cancel')).toBeTruthy() + expect(findButton('Insert image')).toBeUndefined() expect(FakeXMLHttpRequest.instances).toHaveLength(0) }) }) -describe('Dropping a valid file onto the editor opens the uploader already showing the crop step', () => { - it('opens the crop step, and confirming it sends the resized file', async () => { - const wrapper = await mountEditor({ - 'GET /v1/article/12': () => mockResponse(200, articleFixture()), - }) +describe('Dropping a valid file onto the editor opens the uploader on the crop step', () => { + it('looks the hash up, skips the list, and uploads to /v1/media/ from the crop step', async () => { + const wrapper = await mountEditor( + articleHandlers({ + [`GET /v1/media/?hash=${'a'.repeat(64)}`]: () => mockResponse(200, EMPTY_LIBRARY), + }), + ) const file = fileFixture('before.png', 'image/png', 3.9) const dropEvent = new Event('drop', { bubbles: true, cancelable: true }) @@ -219,22 +230,22 @@ describe('Dropping a valid file onto the editor opens the uploader already showi await flushPromises() expect(document.body.textContent).toContain('Crop and resize') - expect(document.body.textContent).not.toContain('Drop an image here') + expect(vi.mocked(global.fetch).mock.calls.map(([url]) => url)).not.toContain( + '/v1/media/?limit=24', + ) findButton('Upload image')?.click() await flushPromises() expect(FakeXMLHttpRequest.instances).toHaveLength(1) - expect(latestXHR().url).toBe('/v1/article/12/image') + expect(latestXHR().url).toBe('/v1/media/') expect((latestXHR().sentBody?.get('file') as File).name).toBe('before.png') }) }) describe('A successful upload inserts the image at the cursor and marks the draft dirty', () => { - it('inserts the image, marks the draft dirty and toasts', async () => { - const wrapper = await mountEditor({ - 'GET /v1/article/12': () => mockResponse(200, articleFixture()), - }) + it('inserts the image with its id, marks the draft dirty and toasts', async () => { + const wrapper = await mountEditor(articleHandlers()) await toolbarImageButton(wrapper)?.trigger('click') await flushPromises() @@ -247,20 +258,25 @@ describe('A successful upload inserts the image at the cursor and marks the draf }) fileInput.dispatchEvent(new Event('change')) await flushPromises() - findButton('Upload image')?.click() await flushPromises() expect(FakeXMLHttpRequest.instances).toHaveLength(1) latestXHR().resolve( 201, - JSON.stringify({ location: '/static/uploadedImages/north-wall.jpg' }), + JSON.stringify({ + id: 13, + filename: 'north-wall.jpg', + url: '/static/uploadedImages/13.jpg', + size: 4089446, + }), ) await flushPromises() const insertedBody = wrapper.findComponent(ArticleEditor).props('modelValue') - expect(insertedBody).toContain('/static/uploadedImages/north-wall.jpg') - expect(insertedBody).toContain('<img src="/static/uploadedImages/north-wall.jpg" alt="">') + expect(insertedBody).toContain( + '<img src="/static/uploadedImages/13.jpg" alt="" data-image-id="13">', + ) expect(wrapper.text()).toContain('Unsaved changes') expect((saveButton(wrapper).element as HTMLButtonElement).disabled).toBe(false) expect(toastAdd).toHaveBeenCalledWith({ diff --git a/web/src/views/__tests__/EditorMediaPicker.spec.ts b/web/src/views/__tests__/EditorMediaPicker.spec.ts new file mode 100644 index 0000000..c076a32 --- /dev/null +++ b/web/src/views/__tests__/EditorMediaPicker.spec.ts @@ -0,0 +1,191 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mount, flushPromises } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import uiPlugin from '@nuxt/ui/vue-plugin' +import router from '@/router' +import { useSessionStore } from '@/stores/session' +import type { Article } from '@/composables/useArticles' +import ArticleEditor from '@/components/ArticleEditor.vue' +import RootComponent from '@/App.vue' +import { fixture } from './mediaTestKit' + +const FIXED_NOW = '2026-09-26T11:02:00Z' + +const BODY_WITH_TWO_IMAGES = + '<p>Intro</p><img src="/media/north-wall.jpg" alt="" data-image-id="1"><img src="/media/logo-renew.svg" alt="" data-image-id="4">' + +function mockResponse(status: number, body: unknown = null) { + return { + ok: status >= 200 && status < 300, + status, + headers: { get: () => null }, + json: async () => body, + } as unknown as Response +} + +function articleFixture(body = BODY_WITH_TWO_IMAGES): Article { + return { + id: 12, + title: 'How we price a bathroom renovation', + body, + is_online: false, + created_at: '2026-03-12T09:14:00Z', + updated_at: FIXED_NOW, + } +} + +function libraryPage() { + const items = fixture().map((item) => + item.id === 2 ? { ...item, url: '/static/uploadedImages/2.webp' } : item, + ) + return { items, next_cursor: 'c24', counts: { all: 48, unused: 2, duplicates: 2 } } +} + +let toastAdd: ReturnType<typeof vi.fn> + +vi.mock('@nuxt/ui/composables', () => ({ + useOverlay: () => ({ create: vi.fn() }), + useToast: () => ({ add: toastAdd }), +})) + +let currentWrapper: Awaited<ReturnType<typeof mount>> | null = null + +async function mountEditor(body = BODY_WITH_TWO_IMAGES) { + setActivePinia(createPinia()) + toastAdd = vi.fn() + const session = useSessionStore() + session.session = { verified: true, username: 'marta' } + session.status = 'ready' + global.fetch = vi.fn(async (url: string) => { + if (url === '/v1/article/12') { + return mockResponse(200, articleFixture(body)) + } + if (url === '/v1/media/?limit=24') { + return mockResponse(200, libraryPage()) + } + throw new Error(`Unexpected fetch call: ${url}`) + }) as unknown as typeof fetch + await router.push('/article/12/edit') + const wrapper = mount(RootComponent, { + attachTo: document.body, + global: { plugins: [router, uiPlugin] }, + }) + currentWrapper = wrapper + await flushPromises() + return wrapper +} + +function findButton(text: string) { + return Array.from(document.body.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === text, + ) +} + +async function openPicker(wrapper: Awaited<ReturnType<typeof mount>>) { + await wrapper + .findAll('button') + .find((button) => button.text() === 'Image') + ?.trigger('click') + await flushPromises() +} + +function tile(filename: string) { + return Array.from(document.body.querySelectorAll<HTMLButtonElement>('[data-picker-tile]')).find( + (element) => element.getAttribute('aria-label')?.startsWith(`${filename},`), + )! +} + +function taggedIds() { + return Array.from(document.body.querySelectorAll<HTMLElement>('[data-picker-tile]')) + .filter((element) => element.textContent?.includes('In this article')) + .map((element) => element.getAttribute('data-image-id')) +} + +afterEach(async () => { + currentWrapper?.unmount() + currentWrapper = null + document.body.innerHTML = '' + await router.push('/login') +}) + +describe('An image is in this article when its node is in the current content', () => { + it('tags images 1 and 4 and no other when the dialog opens', async () => { + const wrapper = await mountEditor() + + await openPicker(wrapper) + + expect(taggedIds()).toEqual(['1', '4']) + }) + + it('counts an insertion that has not been saved yet', async () => { + const wrapper = await mountEditor('<p>Intro</p>') + wrapper.findComponent(ArticleEditor).vm.insertImage('/static/uploadedImages/2.webp', '', 2) + await flushPromises() + + await openPicker(wrapper) + + expect(taggedIds()).toEqual(['2']) + }) + + it('no longer tags an image that was removed from the body', async () => { + const wrapper = await mountEditor() + wrapper.findComponent(ArticleEditor).vm.removeImage(1) + await flushPromises() + + await openPicker(wrapper) + + expect(taggedIds()).toEqual(['4']) + }) +}) + +describe('Insert puts the chosen image in the body with its alt text', () => { + it('inserts it at the cursor, marks the draft dirty, toasts and closes the dialog', async () => { + const wrapper = await mountEditor('<p>Intro</p>') + await openPicker(wrapper) + tile('kitchen-after.webp').click() + await flushPromises() + const alt = document.body.querySelector<HTMLInputElement>( + 'input[placeholder="e.g. North wall of the kitchen before plastering"]', + )! + alt.value = 'A kitchen with new oak worktops and white tiles' + alt.dispatchEvent(new Event('input', { bubbles: true })) + await flushPromises() + + findButton('Insert image')?.click() + await flushPromises() + + const body = wrapper.findComponent(ArticleEditor).props('modelValue') + expect(body).toContain( + '<img src="/static/uploadedImages/2.webp" alt="A kitchen with new oak worktops and white tiles" data-image-id="2">', + ) + expect(wrapper.text()).toContain('Unsaved changes') + expect( + (wrapper.find('[data-testid="save-button"]').element as HTMLButtonElement).disabled, + ).toBe(false) + expect(toastAdd).toHaveBeenCalledWith({ + color: 'success', + icon: 'i-lucide-circle-check', + title: 'Image added', + description: 'Save to keep it in the article.', + }) + expect(document.body.querySelector('[role="dialog"]')).toBeNull() + const methods = vi + .mocked(global.fetch) + .mock.calls.map(([, init]) => (init as RequestInit | undefined)?.method) + expect(methods.every((method) => method === undefined)).toBe(true) + }) + + it('leaves the content and the draft alone when the dialog is cancelled', async () => { + const wrapper = await mountEditor('<p>Intro</p>') + await openPicker(wrapper) + tile('kitchen-after.webp').click() + await flushPromises() + + findButton('Cancel')?.click() + await flushPromises() + + expect(wrapper.findComponent(ArticleEditor).props('modelValue')).not.toContain('<img') + expect(wrapper.text()).not.toContain('Unsaved changes') + expect(toastAdd).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/views/__tests__/EditorView.spec.ts b/web/src/views/__tests__/EditorView.spec.ts index 41fae9c..bdef44f 100644 --- a/web/src/views/__tests__/EditorView.spec.ts +++ b/web/src/views/__tests__/EditorView.spec.ts @@ -3,6 +3,7 @@ import { mount, flushPromises } from '@vue/test-utils' import { createPinia, setActivePinia } from 'pinia' import uiPlugin from '@nuxt/ui/vue-plugin' import router from '@/router' +import { useSeenImagesStore } from '@/stores/seenImages' import { useSessionStore } from '@/stores/session' import type { Article } from '@/composables/useArticles' import ArticleEditor from '@/components/ArticleEditor.vue' @@ -615,8 +616,13 @@ async function requestRemoveImage(wrapper: Awaited<ReturnType<typeof mount>>, im await flushPromises() } -describe('Requesting removal of an inline image opens the confirm dialog', () => { - it('opens ConfirmDialog with the exact copy', async () => { +function requestCalls(wrapper: Awaited<ReturnType<typeof mount>>) { + void wrapper + return vi.mocked(global.fetch).mock.calls.map(([url, init]) => routeFor(url as string, init)) +} + +describe('Requesting removal of an inline image opens the neutral confirm dialog', () => { + it('opens ConfirmDialog with the exact copy and sends nothing', async () => { const wrapper = await mountEditor({ 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), }) @@ -624,27 +630,76 @@ describe('Requesting removal of an inline image opens the confirm dialog', () => await requestRemoveImage(wrapper) const props = overlayMock.getProps() as Record<string, unknown> - expect(props.title).toBe('Remove this image?') + expect(props.title).toBe('Remove this image from the article?') expect(props.description).toBe( - "This image will be removed from the article. This can't be undone.", + "It's removed from this article only. The file stays in your library and in any other article that uses it.", ) expect(props.confirmLabel).toBe('Remove image') expect(props.cancelLabel).toBe('Cancel') - expect(props.pendingLabel).toBe('Removing…') - expect(props.tone).toBe('danger') + expect(props.tone).toBe('neutral') + expect(requestCalls(wrapper)).toEqual(['GET /v1/article/12']) + expect(articleEditorBody(wrapper)).toContain('data-image-id="10"') + }) + + it('previews the image the session has seen with its filename and size', async () => { + const wrapper = await mountEditor({ + 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), + }) + useSeenImagesStore().remember([{ id: 10, filename: 'before.png', size: 4089446 }]) + + await requestRemoveImage(wrapper) + + const props = overlayMock.getProps() as { preview: Record<string, string> } + expect(props.preview).toEqual({ + src: '/static/uploadedImages/before.png', + filename: 'before.png', + size: '3.9 MB', + }) + }) + + it('previews only the thumbnail for an image the session has not seen', async () => { + const wrapper = await mountEditor({ + 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), + }) + + await requestRemoveImage(wrapper) + + const props = overlayMock.getProps() as { preview: Record<string, string> } + expect(props.preview).toEqual({ + src: '/static/uploadedImages/before.png', + filename: undefined, + size: undefined, + }) }) }) -describe('Confirming removes the image, marks the draft dirty, and shows a success toast', () => { - it('removes the image from the content and reads Unsaved changes', async () => { +describe('Cancelling the confirm changes nothing', () => { + it('keeps the image and the draft state when the dialog resolves to cancel', async () => { + const wrapper = await mountEditor({ + 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), + }) + await requestRemoveImage(wrapper) + + overlayMock.resolveOpen(false) + await flushPromises() + + expect(articleEditorBody(wrapper)).toContain('data-image-id="10"') + expect(wrapper.text()).not.toContain('Unsaved changes') + expect(toastAdd).not.toHaveBeenCalled() + expect(requestCalls(wrapper)).toEqual(['GET /v1/article/12']) + }) +}) + +describe('Confirming removes the image from the body only, marks the draft dirty and toasts', () => { + it('removes the image from the content, reads Unsaved changes and sends no request', async () => { const wrapper = await mountEditor({ 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), - 'DELETE /v1/article/12/image/10': () => mockResponse(204), }) await requestRemoveImage(wrapper) const props = overlayMock.getProps() as { action: () => Promise<void> } await props.action() + await flushPromises() expect(articleEditorBody(wrapper)).not.toContain('data-image-id="10"') expect(wrapper.text()).toContain('Unsaved changes') @@ -653,55 +708,9 @@ describe('Confirming removes the image, marks the draft dirty, and shows a succe color: 'success', icon: 'i-lucide-circle-check', title: 'Image removed', - description: 'Save to keep this change.', - }) - }) -}) - -describe('A failed removal keeps the image and shows the dialog inline error', () => { - it.each([ - { name: 'a 404 response', handler: () => mockResponse(404) }, - { name: 'a 500 response', handler: () => mockResponse(500) }, - { - name: 'a network failure', - handler: () => Promise.reject(new TypeError('Failed to fetch')), - }, - ])('rejects with the inline error message for $name', async ({ handler }) => { - const wrapper = await mountEditor({ - 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), - 'DELETE /v1/article/12/image/10': handler, - }) - await requestRemoveImage(wrapper) - - const props = overlayMock.getProps() as { action: () => Promise<void> } - await expect(props.action()).rejects.toThrow( - "Couldn't remove the image, so it's still there. Try again in a moment.", - ) - - expect(articleEditorBody(wrapper)).toContain('data-image-id="10"') - expect(wrapper.text()).not.toContain('Unsaved changes') - }) -}) - -describe('Confirming a removal started by Backspace behaves exactly like confirming one started by the button', () => { - it('removes the image, marks the draft dirty, and shows the same success toast', async () => { - const wrapper = await mountEditor({ - 'GET /v1/article/12': () => mockResponse(200, articleWithImageFixture()), - 'DELETE /v1/article/12/image/10': () => mockResponse(204), - }) - await requestRemoveImage(wrapper) - - const props = overlayMock.getProps() as { action: () => Promise<void> } - await props.action() - - expect(articleEditorBody(wrapper)).not.toContain('data-image-id="10"') - expect(wrapper.text()).toContain('Unsaved changes') - expect(toastAdd).toHaveBeenCalledWith({ - color: 'success', - icon: 'i-lucide-circle-check', - title: 'Image removed', - description: 'Save to keep this change.', + description: "It's still in your library.", }) + expect(requestCalls(wrapper)).toEqual(['GET /v1/article/12']) }) }) diff --git a/web/visual/states.ts b/web/visual/states.ts index 1c74e32..81230c4 100644 --- a/web/visual/states.ts +++ b/web/visual/states.ts @@ -186,10 +186,17 @@ const editorArticle = { updated_at: editorNow, } +const emptyLibrary = { + items: [], + next_cursor: null, + counts: { all: 0, unused: 0, duplicates: 0 }, +} + function editorApi(overrides: Record<string, ApiMock> = {}): Record<string, ApiMock> { return { 'GET /v1/auth/me': verifiedSession, 'GET /v1/article/12': { status: 200, body: editorArticle }, + 'GET /v1/media/': { status: 200, body: emptyLibrary }, ...overrides, } } @@ -384,7 +391,7 @@ async function freezeUploadProgress(page: Page, loaded: number, total: number) { const originalSend = proto.send as (...args: unknown[]) => void proto.send = function (this: XMLHttpRequest, ...args: unknown[]) { const url = (this as unknown as { __imageUrl?: string }).__imageUrl ?? '' - if (url.includes('/image')) { + if (url.includes('/v1/media')) { setTimeout(() => { const event = new Event('progress') Object.assign(event, { loaded, total, lengthComputable: true }) @@ -1583,6 +1590,253 @@ const mediaStates: Record<string, VisualState> = { Offline: mediaState(mediaApi(), goOffline), } +const pickerIntro = + 'A bathroom is priced by what has to happen behind the walls before anything gets tiled.' +const pickerOutro = 'We don’t add a margin on materials you choose yourself.' +const invisibleImage = '/static/uploadedImages/blank.svg' +const invisibleImageSvg = '<svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"/>' +const boardImageSvgFor = (width: number, height: number) => + [ + `<svg xmlns="http://www.w3.org/2000/svg" width="${width}" height="${height}" viewBox="0 0 672 378" preserveAspectRatio="xMidYMid slice">`, + '<style>.tone{fill:#e8dfd1}.ink{color:#7d6d56}@media (prefers-color-scheme:dark){.tone{fill:#d4c5b0}}</style>', + '<defs><pattern id="hatch" width="12" height="12" patternUnits="userSpaceOnUse" patternTransform="rotate(45)">', + '<rect width="1" height="12" fill="rgb(128 128 128 / 0.12)"/></pattern></defs>', + '<rect class="tone" width="672" height="378"/>', + '<rect fill="url(#hatch)" width="672" height="378"/>', + '<g class="ink" opacity="0.55" transform="translate(316 160) scale(1.6667)" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">', + '<rect x="3" y="3" width="18" height="18" rx="2"/><circle cx="9" cy="9" r="2"/><path d="m21 15-3.086-3.086a2 2 0 0 0-2.828 0L6 21"/></g>', + '</svg>', + ].join('') + +const boardImageMaxWidth = 672 +const boardImageSideSpace = 32 + +function boardImageFor(page: Page) { + const viewport = page.viewportSize() + const width = Math.min( + boardImageMaxWidth, + (viewport?.width ?? boardImageMaxWidth) - boardImageSideSpace, + ) + return boardImageSvgFor(width, Math.round((width * 9) / 16)) +} +const boardImage = '/static/uploadedImages/board-image.svg' + +function pickerBody(image: string | null, ids: number[] = []) { + const nodes = ids.map( + (id) => `<img src="${image ?? invisibleImage}" alt="" data-image-id="${id}">`, + ) + return `<p>${pickerIntro}</p>${nodes.join('')}<p>${pickerOutro}</p>` +} + +const pickerCounts = { all: 48, unused: 2, duplicates: 2 } +const pickerFirstPageCursor = 'c24' +const pickerSearchTerm = 'wall' + +function pickerList(url: URL) { + if (url.searchParams.has('hash')) { + return mediaList(mediaItems([1]), pickerCounts) + } + if (url.searchParams.has('q')) { + return { items: [], next_cursor: null, counts: pickerCounts } + } + return { items: mediaItems(), next_cursor: pickerFirstPageCursor, counts: pickerCounts } +} + +function pickerApi( + overrides: Record<string, ApiMock> = {}, + body = pickerBody(null, [1, 4]), +): Record<string, ApiMock> { + return { + 'GET /v1/auth/me': mediaSession, + 'GET /v1/article/12': { status: 200, body: { ...editorArticle, body } }, + 'GET /v1/media/': { status: 200, bodyFor: pickerList }, + ...overrides, + } +} + +function pickerState( + steps?: (page: Page) => Promise<void>, + api: Record<string, ApiMock> = pickerApi(), +): VisualState { + return { + path: '/article/12/edit', + api, + steps: async (page) => { + await stubEditorImages(page) + await steps?.(page) + }, + } +} + +async function openPickerTab(page: Page, name: 'Upload' | 'Library') { + await openImageUploader(page) + await page.getByRole('tab', { name, exact: true }).click() +} + +async function waitForPickerTiles(page: Page) { + await page.locator('[data-picker-tile]').first().waitFor() +} + +async function selectPickerTile(page: Page, filename: string) { + await waitForPickerTiles(page) + await page.locator(`[data-picker-tile][aria-label^="${filename},"]`).click() + await page.getByTestId('picker-panel').waitFor() +} + +async function insertFromLibrary(page: Page, filename: string) { + await openImageUploader(page) + await selectPickerTile(page, filename) + await page.getByRole('button', { name: 'Insert image', exact: true }).click() + await page.getByText('Save to keep it in the article.').first().waitFor() +} + +async function selectEditorImage(page: Page) { + await page.locator('[data-testid="article-editor-image"] img').first().click() + await page.getByRole('button', { name: 'Remove image', exact: true }).waitFor() +} + +async function stubEditorImages(page: Page) { + await page.route('**/static/uploadedImages/blank.svg', (route) => + route.fulfill({ status: 200, contentType: 'image/svg+xml', body: invisibleImageSvg }), + ) + await page.route('**/static/uploadedImages/board-image.svg', (route) => + route.fulfill({ status: 200, contentType: 'image/svg+xml', body: boardImageFor(page) }), + ) + await page.reload() + await page.waitForFunction( + () => (document.querySelector('#app')?.textContent ?? '').trim().length > 0, + ) +} + +async function stubBoardImage(page: Page) { + await page.route('**/media/north-wall.jpg', (route) => + route.fulfill({ status: 200, contentType: 'image/svg+xml', body: boardImageFor(page) }), + ) +} + +async function browseLibraryThenClose(page: Page) { + await openImageUploader(page) + await waitForPickerTiles(page) + await page.keyboard.press('Escape') + await page.getByRole('dialog').waitFor({ state: 'hidden' }) +} + +async function requestRemoveFromEditor(page: Page) { + await browseLibraryThenClose(page) + await selectEditorImage(page) + await page.getByRole('button', { name: 'Remove image', exact: true }).click() + await page.getByRole('alertdialog').waitFor() +} + +const simplifiedEditorReason = + 'The board draws a simplified editor (centred 680 px column, no toolbar, no details panel, fixed image placeholder) while the real editor is left-aligned with a toolbar, a taller header and a details panel, so the image block and page chrome sit elsewhere' +const simplifiedEditorMaxPercent = 40 + +function withSimplifiedEditorException(state: VisualState): VisualState { + return { + ...state, + exception: { + maxPercent: simplifiedEditorMaxPercent, + reason: simplifiedEditorReason, + approvedBy: 'user', + }, + } +} + +const pickerStates: Record<string, VisualState> = { + 'Upload tab': pickerState(async (page) => { + await openPickerTab(page, 'Upload') + }), + 'Upload tab: already in your library': { + ...pickerState(async (page) => { + await openPickerTab(page, 'Upload') + await chooseImageFile(page, 'north-wall.jpg', 'image/jpeg', mib(3.9)) + await page.getByText('Already in your library').waitFor() + }), + exception: { + maxPercent: 20, + reason: 'The board buttons collapse to about 22 px on mobile (flex:1 in a column) while the feature requires 44 px full-width stacked buttons, which makes the modal 31 px taller', + approvedBy: 'user', + }, + }, + 'Library: loading': pickerState( + async (page) => { + await openImageUploader(page) + await page.getByTestId('picker-skeleton-tile').first().waitFor() + }, + pickerApi({ 'GET /v1/media/': { status: 200, hang: true } }), + ), + 'Library: nothing selected': pickerState(async (page) => { + await openImageUploader(page) + await waitForPickerTiles(page) + }), + 'Library: selected': pickerState(async (page) => { + await openImageUploader(page) + await selectPickerTile(page, 'kitchen-after.webp') + }), + 'Library: alt text entered': pickerState(async (page) => { + await openImageUploader(page) + await selectPickerTile(page, 'kitchen-after.webp') + await page + .getByPlaceholder('e.g. North wall of the kitchen before plastering') + .fill('A kitchen with new oak worktops and white tiles') + }), + 'Library: loading more': pickerState(async (page) => { + await openImageUploader(page) + await waitForPickerTiles(page) + await page.route(/\/v1\/media\/\?.*cursor=/, () => undefined) + await page.getByRole('button', { name: 'Load more', exact: true }).click() + await page.getByTestId('picker-skeleton-tile').first().waitFor() + }), + 'Library: empty': pickerState( + async (page) => { + await openPickerTab(page, 'Library') + await page.getByText('Your library is empty').waitFor() + }, + pickerApi({ 'GET /v1/media/': { status: 200, body: emptyLibrary } }), + ), + 'Library: no results': pickerState(async (page) => { + await openImageUploader(page) + await waitForPickerTiles(page) + await page.getByLabel('Search by filename').fill(pickerSearchTerm) + await page.getByText(`No images match "${pickerSearchTerm}"`).waitFor() + }), + 'Library: failed to load': pickerState( + async (page) => { + await openImageUploader(page) + await page.getByText("Couldn't load your library").waitFor() + }, + pickerApi({ 'GET /v1/media/': { status: 500 } }), + ), + Inserted: withSimplifiedEditorException( + pickerState( + async (page) => { + await stubBoardImage(page) + await page.getByText('behind the walls').click() + await page.keyboard.press('End') + await insertFromLibrary(page, 'north-wall.jpg') + await page.waitForTimeout(400) + await selectEditorImage(page) + }, + pickerApi({}, pickerBody(null)), + ), + ), + 'Remove image: confirm': withSimplifiedEditorException( + pickerState(requestRemoveFromEditor, pickerApi({}, pickerBody(boardImage, [1]))), + ), + Removed: pickerState( + async (page) => { + await requestRemoveFromEditor(page) + await page + .getByRole('alertdialog') + .getByRole('button', { name: 'Remove image', exact: true }) + .click() + await page.getByText("It's still in your library.").first().waitFor() + }, + pickerApi({}, pickerBody(boardImage, [1])), + ), +} + export const visualStates: Record<string, Record<string, VisualState>> = { profile: { 'Account menu': { @@ -2306,17 +2560,30 @@ export const visualStates: Record<string, Record<string, VisualState>> = { path: '/article/12/edit', now: editorNow, api: editorApi({ - 'POST /v1/article/12/image': { + 'POST /v1/media/': { status: 201, - body: { location: '/static/uploadedImages/north-wall.jpg' }, + body: { + id: 13, + filename: 'north-wall.jpg', + url: '/static/uploadedImages/north-wall.jpg', + size: mib(3.9), + content_type: 'image/jpeg', + width: 4032, + height: 3024, + sha256: 'a'.repeat(64), + uploaded_by: 'Marta Alves', + created_at: editorNow, + used_in: 0, + duplicate: false, + }, }, }), steps: async (page) => { await mockUploadedImage(page) await page.getByText('add a margin on materials').click() await page.keyboard.press('Home') - await openImageUploader(page) - await chooseImageFile(page, 'north-wall.jpg', 'image/jpeg', mib(3.9)) + await openCropStep(page, 'north-wall.jpg', 'image/jpeg', 4032, 3024, mib(3.9)) + await page.getByRole('dialog').getByRole('button', { name: 'Upload image' }).click() await page.getByText('Save to keep it in the article.').first().waitFor() await page.waitForTimeout(650) }, @@ -2514,4 +2781,5 @@ export const visualStates: Record<string, Record<string, VisualState>> = { }, }, media: mediaStates, + 'media-picker': pickerStates, }