You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Confirm yellow warning stays after the valid file finishes uploading
Click dismiss on the rejected entry — entry removed, yellow warning disappears
Multiple rejected files
Drop two unsupported files
Both rejected entries visible, yellow warning present
Dismiss one — yellow warning stays (one entry remains)
Dismiss the second — yellow warning disappears
New drop clears old rejections
Drop one unsupported file — rejected entry visible, yellow warning present
Drop one supported file — rejected entry from the previous drop disappears immediately and yellow warning clears immediately on drop (before upload completes)
Added dismissValidationErrors() and dismissFile() actions; both registered in makeObservable; processDrop now clears old validation errors before adding new ones
File:packages/pluggableWidgets/file-uploader-web/src/components/ActionsBar.tsx line 88 Note:DismissActionsBar uses translations.get("removeButtonTextMessage") for the dismiss button title. This means the tooltip/accessible name is "Remove" (the same label as the permanent-file remove button). For validation-error entries there is no file object to remove — the action is dismissing an error — so the label is semantically misleading. Consider a dedicated dismissButtonTextMessage XML property (or at least re-using a more neutral existing key) so screen-reader users get an accurate description of the action.
⚠️ Low — makeStore() bypasses makeObservable, so MobX reactivity is untested
File:packages/pluggableWidgets/file-uploader-web/src/stores/__tests__/FileUploaderStore.spec.ts lines 14-29 Note: The helper constructs a FileUploaderStore instance via Object.create(…) + Object.assign(…), skipping the constructor and therefore makeObservable. The tests only assert array contents (.files), so they still pass, but any test that needed to verify that a mutation is wrapped in a MobX action (strict-mode violation) or that a computed updates would be silently incorrect. This is fine for the narrow coverage here, but add a comment explaining the bypass so future contributors don't copy the pattern for observer/computed tests.
File:packages/pluggableWidgets/file-uploader-web/src/stores/__tests__/FileUploaderStore.spec.ts line 37 Note:(f as any).fileStatus = "done" bypasses MobX observable tracking. In tests that don't run with MobX strict mode this is harmless, but if strict mode is ever enabled these tests would throw. Prefer runInAction(() => { f.fileStatus = "done"; }).
Positives
Both new store methods (dismissValidationErrors, dismissFile) are correctly registered as action in makeObservable — no observable mutation escapes MobX tracking in production code.
Auto-clearing on successful upload (dismissValidationErrors() inside runInAction) is placed in the same transaction as the status update, keeping the UI update atomic.
SCSS change correctly scopes the opacity to only the preview/content cells, leaving the actions column fully opaque — a clean improvement over the previous display: none approach.
Test file for DismissActionsBar exercises the full render → click → spy chain via RTL, which is the correct pattern for this repo.
CHANGELOG entry is present and follows Keep a Changelog format.
New unit tests for dismissFile / dismissValidationErrors / processDrop
Skipped (out of scope): dist/, pnpm-lock.yaml
⚠️ CI checks could not be retrieved during this review — verify all checks are green before merging.
Findings
⚠️ Low — Missing afterEach mock cleanup in new test files
File:packages/pluggableWidgets/file-uploader-web/src/stores/__tests__/FileStore.spec.ts line 42 File:packages/pluggableWidgets/file-uploader-web/src/components/__tests__/DismissActionsBar.spec.tsx line 52 Note: Neither file resets module-level mocks between tests. saveFile and fetchMxObject are set with mockResolvedValue inside a test body in FileStore.spec.ts and will bleed into any tests added later in the same suite. Add a cleanup to both files:
afterEach(()=>{jest.clearAllMocks();});
⚠️ Low — Manual DynamicValue objects instead of builders in DismissActionsBar.spec.tsx
File:packages/pluggableWidgets/file-uploader-web/src/components/__tests__/DismissActionsBar.spec.tsx line 18 Note:makeFakeProps constructs DynamicValue<string> fields by hand ({ value: v, status: "available" }). The repo standard is to use DynamicValueBuilder from @mendix/widget-plugin-test-utils, which correctly shapes the type and supports status variations:
import{dynamicValue}from"@mendix/widget-plugin-test-utils";removeButtonTextMessage: dynamicValue("Remove"),downloadButtonTextMessage: dynamicValue("Download"),// etc.
Positives
dismiss, dismissFile, and dismissValidationErrors are all properly registered as action in their respective makeObservable calls — no stray mutations outside action context.
The isDragActive && isDragReject fix in Dropzone.tsx is a clean, minimal correction that prevents the reject-state indicator from persisting after the user stops dragging.
processDrop correctly calls dismissValidationErrors() before processing the new drop, so stale rejected entries are cleared atomically on each new batch.
Test coverage is thorough: per-file dismiss, multi-file dismiss, last-validation-error clears the message, no-op when file not in list, and drop-with-rejections vs drop-without-rejections are all covered.
CHANGELOG entry is present in [Unreleased] with a clear user-facing description.
New unit tests for dismissFile and processDrop error-message behaviour
Skipped (out of scope): dist/, pnpm-lock.yaml
Findings
⚠️ Low — dismissFile does not clear message when a non-validation-error file is dismissed alongside the last rejected file
File:packages/pluggableWidgets/file-uploader-web/src/stores/FileUploaderStore.ts line 151–156 Problem:dismissFile only calls setMessage() when no remaining files have validationError status. This is correct for the happy path, but if someone were to extend the method to also dismiss non-error files (e.g. a future remove path), the logic would silently skip clearing the message. More concretely right now: if the last validationError file is dismissed and a done file with a lingering warning (set before the done transition) is still present, the message will correctly clear — but the asymmetry between dismissValidationErrors (always clears) and dismissFile (only clears when no validation errors remain) can be surprising and isn't covered by a test.
The existing test suite covers the "no-op when file not in list" case but doesn't assert the errorMessage is unchanged when a done file remains alongside the dismissed error file — a subtle case.
Fix: Add a test asserting that errorMessage is preserved when dismissing a validationError file while another validationError file still exists (already covered), and add a complementary case verifying errorMessage is cleared when the only remaining files after dismissal are non-error files:
it("clears errorMessage when dismissed file was the last validationError but done files remain",()=>{conststore=makeStore();store.errorMessage="Some files may not be uploadable.";consterrorFile=makeValidationErrorFile(store);constdoneFile=makeDoneFile(store);store.files=[errorFile,doneFile];store.dismissFile(errorFile);expect(store.errorMessage).toBeUndefined();expect(store.files).toHaveLength(1);});
⚠️ Low — makeStore() bypasses makeObservable, so MobX reactivity is not tested
File:packages/pluggableWidgets/file-uploader-web/src/stores/__tests__/FileUploaderStore.spec.ts line 14–29 Problem: The store is created via Object.create(FileUploaderStore.prototype) + Object.assign, which skips the constructor and therefore skips makeObservable. Mutating store.files directly in tests works today because the tests only assert final values, not reactive updates. If any future test needs to verify that a MobX reaction fires, this helper will silently fail. The errorMessage field also won't be observable in this setup, so the store.errorMessage = "..." assignments are plain property sets.
This is a pre-existing test-architecture issue that this PR introduces — not a blocker, but worth noting.
Fix: Consider constructing a minimal real store using actual FileUploaderContainerProps-like input and TranslationsStore, similar to how DismissActionsBar.spec.tsx does it with makeFakeProps(). This makes the store fully wired with MobX.
⚠️ Low — actions prop is not forwarded into DismissActionsBar
File:packages/pluggableWidgets/file-uploader-web/src/components/ActionsBar.tsx line 13–16 Problem: The early-return on validationError means custom actions (from Studio Pro) are completely ignored for rejected files, with no comment explaining this is intentional. If widget consumers configure custom buttons, they will silently disappear for validation-errored entries.
Fix: If this is intentional design (only the dismiss button for rejected files, regardless of custom buttons), add a brief inline comment so future maintainers don't accidentally break this assumption:
// Validation-errored files always show only the dismiss button; custom actions are not applicable.if(store.fileStatus==="validationError"){return<DismissActionsBarstore={store}/>;}
Positives
The isDragActive && isDragReject fix in Dropzone.tsx is a clean, targeted correction — react-dropzone reports isDragReject even when not dragging (after a rejected drop), so gating on isDragActive prevents the yellow warning from rendering on hover after a prior rejection.
dismissValidationErrors() is correctly called before setting a new message in processDrop, ensuring stale error entries from the previous drop batch are always cleared atomically.
Both dismiss in FileStore and dismissFile in FileUploaderStore are properly registered in makeObservable as action, keeping MobX state mutations correctly batched.
Test coverage is thorough: edge cases (no-op dismiss, partial dismiss, last-file-dismissed) are all exercised.
CHANGELOG entry is present and well-worded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
yordan-st
force-pushed
the
fix/WC-3363_fileuploader-close-warning
branch
from
Pull request type
Bug fix (non-breaking change which fixes an issue)
Description
What should be covered while testing?
Single rejected file
Mixed batch (valid + invalid)
Multiple rejected files
New drop clears old rejections