[Translations] Fix: search & field filters in Translations sidebar don't trigger on Enter/search icon - #4012
Conversation
SearchTermControl only wired onChange, but antd's Input.Search fires onSearch (not onChange) for the search icon click and the Enter key. As a result, the search & filter sidebar required using the "Apply" button, silently ignoring the search icon and Enter key. Add an onSearch handler that applies the term immediately, mirroring the working element listing's SearchTermFilter.
Text field filters (e.g. the "German" column) only wrote into the sidebar's draft store on blur, so a value typed there was silently dropped unless the user also clicked the separate "Apply" button. Give the field-filter dynamic type a search input and thread an optional onCommit callback through DynamicFilterProvider and FieldFilters so a committed value (Enter or the search icon) is written straight to the applied filter store, matching the existing "Apply" button behavior. Also stretch the input to the full sidebar width to match the main search field.
There was a problem hiding this comment.
Pull request overview
Enables immediate submission of translation search and text field filters via Enter or the search icon.
Changes:
- Adds commit handling across dynamic and field-filter components.
- Uses
SearchInputfor text field filters. - Applies committed translation filters immediately.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
filter-tab.tsx |
Connects field-filter commits. |
use-translations-field-filter-editor.ts |
Synchronizes committed field filters. |
filters.tsx |
Handles top-level search submission. |
dynamic-type-field-filter-text-component.tsx |
Adds searchable text-filter input. |
field-filters.tsx |
Propagates filter commits. |
dynamic-filter-provider.tsx |
Adds commit support to filter context. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Committing the search term or a field filter only wrote that single key into the applied filter store, so any other still-unsaved draft change (e.g. a field filter typed but not yet applied) was silently dropped instead of being applied together, unlike clicking "Apply" which applies the entire draft snapshot at once. Apply the full current draft values on commit, overriding just the field being committed to avoid reading a stale React state value for it.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
assets/js/src/core/modules/element/dynamic-types/definitions/field-filters/components/dynamic-type-field-filter-text-component.tsx:32
- Use the
searchValuesupplied bySearchInputhere. The clear control invokesonSearchwith an empty value in the same event asonChange, before_valuehas re-rendered, so this closure can recommit the previous text and leave the filter applied instead of clearing it.
onSearch={ () => { commit(_value) } }
…state
The field filter's SearchInput read the committed value from local
component state (_value) rather than the value onSearch itself
received. Clicking the clear (x) icon fires onChange('') and
onSearch('') in the same event before the state update from onChange
has re-rendered, so the stale closure could recommit the previous
text and leave the filter applied instead of clearing it.
Commit the value passed into onSearch directly to avoid the race.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (2)
assets/js/src/core/components/field-filters/field-filters.tsx:67
- This newly added direct
undefinedcomparison bypasses the project's type-safe check convention. Invoke the optional callback directly (or useisUndefined) so this branch follows the required nullability pattern.
if (onCommit !== undefined) {
onCommit(updatedData)
}
assets/js/src/core/components/dynamic-filter/provider/dynamic-filter-provider.tsx:65
- This newly added direct
undefinedcomparison bypasses the project's type-safe check convention. Invoke the optional callback directly (or useisUndefined) so this branch follows the required nullability pattern.
if (onCommit !== undefined) {
onCommit(data)
}
…d onCommit FieldFilters has six hosts (element listing, notes & events, notifications, recycle bin, reports, translations) and all of them collect draft values behind their own "Apply" button. Switching the shared text field filter to SearchInput gave all six a search icon and Enter handling, but only translations wires onCommit -- in the other five commit() falls through to onChange, i.e. a draft write, so the new control looked actionable while doing no more than blur already did. Make commit() optional and build it only when the host passed onCommit; FieldFilters forwards onCommit only when it has one. The text filter renders SearchInput when commit is available and the previous plain Input otherwise, so the five draft-only hosts keep their markup unchanged and each opts in later by passing onCommit. This also drops an accidental BC break: commit was a required member of the SDK-exported DynamicFilterData, which breaks external code constructing that object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Commit currently emits duplicate change callbacks, and the new reusable component behavior lacks its required Storybook coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 2
- Review effort level: Balanced
The search-input treatment did not fit a field filter row: a magnifier addon and a clear icon on every text filter read as a second, competing search box next to the sidebar's own search field. Drop SearchInput and keep the antd Input the component always used, wiring onPressEnter instead. Enter applies immediately where the host exposes commit() and otherwise behaves like leaving the field, so the rendered markup is now unchanged in every host and no styling or e2e selector shifts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DynamicFilterProvider.commit called onChange and then onCommit, and the onCommit consumer in FieldFilters calls setData itself, which fires onChange again. A single commit therefore delivered the same value to the host's onChange twice before onCommit, doubling the state updates and breaking the one-callback-per-interaction expectation. Drop the onChange call from commit(); FieldFilters.onFilterCommit already reports the change through setData before invoking onCommit. Hosts without an onCommit handler never reach commit(), so their onChange path is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pers The classification-store id-plus-config comparison was written out four times: three findIndex lookups (change, commit, language selection) and once more inverted inside onRemoveClick. onFilterCommit in particular was a character-for-character copy of onFilterChange. Pull the comparison into isSameFilter() and the array patch into withUpdatedFilter(), and express all four handlers through them. Behaviour is unchanged: the same entry is matched and the same object spread applied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
🟡 Changes recommended
Field filters still use a plain input without the required clickable search icon or full-width search control.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
Thanks a lot for the report and the fix. I also pushed commits to your branch to change one thing: the search control inside the field filter rows.
Why. DynamicTypeFieldFilterTextComponent is not specific to Translations - it powers the text field filter in six sidebars. Switching it to SearchInput added a magnifier icon and a clear button to all six. There were two issues with that:
- Only Translations wires onCommit. In the other five cases, commit() falls back to onChange, which simply writes the draft value. As a result, the button looked actionable while doing no more than leaving the field already did. A control that promises an action it cannot actually perform is worse than no control.
- Visually it reads as a second search box competing with the sidebar's own search field right above it, once per filter row. It doesn't match how the rest of the filter rows look.
What it does now. The component keeps the plain antd Input it always used and reacts to onPressEnter instead: Enter applies immediately where the host exposes commit(), and behaves like leaving the field where it doesn't. The rendered markup is therefore unchanged in every host - no styling or selector shifts - and any sidebar can opt into immediate-apply later by passing onCommit.
One consequence worth stating plainly: the "search icon and full sidebar width" part of #354's expected behaviour is intentionally not implemented. Enter and the top search field cover the functional half; the per-row icon we're deliberately leaving out.
I hope this explains the reasoning behind the changes.
I will merge the PR soon.
Summary
DynamicFilterProvider → FieldFilters → the translations field-filter hook, so Enter/search-icon immediately applies the filter, and stretched the input to full sidebar width.
Changes
Fixed state:
Fixes pimcore/platform-version#354
The issue was also reported as an enterprise issue: PEES-1392