Skip to content

[Query Builder] Refactor, bug fixing and test coverage improved - #17706

Merged
kdinev merged 17 commits into
masterfrom
ipetrov/query-builder-coverage
Oct 17, 2026
Merged

kdinev merged 17 commits into
masterfrom
ipetrov/query-builder-coverage

Conversation

@ivanvpetrov

@ivanvpetrov ivanvpetrov commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Description

  • Raises Query Builder unit test coverage from 88.7% to ~98.4% (lines).
  • Fixes several bugs that were exposed during the refactor, with tests for each.
  • Removes dead code.

Fixes

  • Keyboard drag:
    • Pressing Space did nothing, because the listener checked for 'Space' while the browser reports ' '.
    • Pressing Space or Enter before choosing a drop location deleted the dragged expression. Both keys now drop only once a drop ghost is shown.
    • ArrowUp/ArrowDown and the Space/Enter key that drops now call preventDefault() (held keys included), so the page/container no longer scrolls.
    • Tab and Shift+Tab are ignored while the drop ghost is shown, so focus stays on it until the condition is dropped (Enter/Space) or the drag is cancelled (Escape). Previously they cancelled the drag and focus was lost.
  • Mouse drag + keyboard:
    • While a chip is held with the mouse and its drop ghost is shown, tabbing to another chip's drag indicator let ArrowUp/ArrowDown move the drop ghost without moving the mouse, and Space/Enter drop the mouse-dragged chip there.
    • Focusing another chip's drag indicator during a mouse drag (before a drop ghost appears) started a keyboard drag of that other chip.
    • Added a guard that prevents keyboard drag while a mouse drag is in progress.
  • Time fields: selecting a time field without editorOptions threw an error (editorOptions!.dateTimeFormat, now ?.).

Unreachable code removed (all private/protected/@hidden @internal; IgxQueryBuilderTreeComponent is not exported):

  • Unused getters (editingInputsContainer, currentGroupButtonsContainer), endGroup(), onKeyDown()
  • addRootAndGroupButton @ViewChild, whose template ref no longer exists (setAddButtonFocus keeps its behavior)
  • _expandedExpressions, which was never written to
  • String handling for _selectedReturnFields, which is always an array; the type is narrowed to string[]
  • The unreachable 'wrong key' branch in arrowDrag; the key is typed as 'ArrowUp' | 'ArrowDown'
  • The commented-out Tab-blocking code in the drag service, now implemented (see Fixes)

Tests

  • New specs for: add-menu flows (add group, add-mode transitions, nested query), focus/blur, return fields, time/dateTime/currency/percent/untyped fields, an entity without fields, return-field chip text, commit()/setAddButtonFocus(), AND/OR switch before the first commit, and resource strings.
  • Drag and drop: no-op events, re-targeting, release outside a drop area, focus-out cancel, focus moving to the drop ghost, keyboard input ignored during a mouse drag, default prevented for keyboard-drag keys, and Tab blocked during a keyboard drag.
  • Re-enabled the disabled xit drop-ghost spec. Its hard-coded pointer offsets missed the drop target, so it now moves the pointer to the measured "Add condition" button.

Follow-ups / known limitations

Motivation / Context

Improve Query Builder test coverage.

Type of Change (check all that apply):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog
  • Skills/Agents

Component(s) / Area(s) Affected:

Query Builder (also used by grid advanced filtering)

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

All query-builder specs and the non-grid suite pass. All specs that open grid advanced filtering pass (390). npm run lint:lib reports no errors.

Test Configuration:

  • Angular version: 22
  • Browser(s): Chrome Headless
  • OS: Windows 11

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified

Copilot AI balanced review requested due to automatic review settings October 5, 2026 11:10

This comment was marked as resolved.

Space/Enter before any arrow key deleted the dragged expression.

Also strengthen weak specs flagged in review.

This comment was marked as resolved.

This comment was marked as resolved.

@ivanvpetrov
ivanvpetrov requested a balanced review from Copilot October 6, 2026 14:34

This comment was marked as resolved.

@ivanvpetrov ivanvpetrov changed the title test(query-builder): coverage improved Query Builder: Refactor, bug fixing and test coverage improved Oct 6, 2026
@ivanvpetrov ivanvpetrov changed the title Query Builder: Refactor, bug fixing and test coverage improved [Query Builder] Refactor, bug fixing and test coverage improved Oct 6, 2026
@ivanvpetrov
ivanvpetrov requested a balanced review from Copilot October 6, 2026 14:59

This comment was marked as low quality.

@ivanvpetrov
ivanvpetrov marked this pull request as ready for review October 8, 2026 13:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation matches the described fixes and includes comprehensive regression coverage with no unresolved issues found.

0 open findings

🧠 Review effort: Balanced

@kdinev
kdinev enabled auto-merge (squash) October 17, 2026 12:39
@kdinev
kdinev merged commit 042290c into master Oct 17, 2026
6 checks passed
@kdinev
kdinev deleted the ipetrov/query-builder-coverage branch October 17, 2026 12:52
kdinev added a commit that referenced this pull request Oct 17, 2026
…itest

Brings in #17706 (query builder refactor and tests). The query builder
spec keeps this branch's Vitest version; the PR's new tests are ported
there in the follow-up working-tree changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants