feat(manager): Remove deprecated toolbar filter ids in SettingsGrid - #16976
Open
Ibochkarev wants to merge 1 commit into
Open
feat(manager): Remove deprecated toolbar filter ids in SettingsGrid#16976Ibochkarev wants to merge 1 commit into
Ibochkarev wants to merge 1 commit into
Conversation
Drop the global Ext id config on the namespace/area filter combos and rename itemId 'filter-ns' to 'filter-namespace', matching the 3.1 removal noted in the deprecated JSDoc and the convention already used by other grids (lexicon, user group namespace). Reuse the existing getFilterComponent helper in MODx.window.CreateSetting instead of duplicating getTopToolbar().getComponent(...) lookups. Refs modxcms#16972
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed and why
The namespace and area filter combos in
MODx.grid.SettingsGridused global Extids (modx-filter-namespace,modx-filter-area). Two settings grids on the same page collide on those ids. The source already flagged this for removal in 3.1:This PR drops the
idconfig from both combos and renamesitemIdfilter-nstofilter-namespace, matching the naming already used in the lexicon and user group namespace grids.MODx.window.CreateSettingnow reads filter values throughgetFilterComponent()instead ofgetTopToolbar().getComponent(...), so the lookup goes through the same helper the rest of the codebase already uses.How to test
Open System Settings, a Context settings tab, and a User/User Group ACL settings grid. Namespace, area, and query filters should work, Clear Filters should reset them, and the Create Setting window should prefill namespace/area from the active grid filters.
Related issue(s)/PR(s)
Resolves #16972
Compatibility notes
Manager-only change, no platform or environment dependency. Anything relying on the removed global ids (
modx-filter-namespace,modx-filter-area) viaExt.getCmpwould break, but a repo-wide search found no such references.Breaking change assessment
No public API or return type changes.
itemIdvalues are internal to the grid's toolbar and were already accessed throughgetComponent()/getFilterComponent()rather than exposed as public contract, so this is safe for 3.x consumers.Test coverage
No automated tests added; this is a JS/ExtJS UI change with no existing test harness for grid toolbars. Verified manually per the steps above.
Contributors
@Ibochkarev
AI tool use
Cursor AI helped implement the rename and the
getFilterComponent()refactor, and drafted this PR description.