Skip to content

Fix LT-22450: Filtering causes wrong word to appear in Pop Up#748

Merged
jtmaxwell3 merged 1 commit intomainfrom
LT-22450
Mar 10, 2026
Merged

Fix LT-22450: Filtering causes wrong word to appear in Pop Up#748
jtmaxwell3 merged 1 commit intomainfrom
LT-22450

Conversation

@jtmaxwell3
Copy link
Collaborator

@jtmaxwell3 jtmaxwell3 commented Mar 9, 2026

This fixes https://jira.sil.org/browse/LT-22450. The filter needs to be cleared even when doing a pop up. This was introduced as part of https://jira.sil.org/browse/LT-22358, but it does not appear to be necessary. LT-22358 should be retested.


This change is Reviewable

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

NUnit Tests

    1 files  ±0      1 suites  ±0   5m 33s ⏱️ -4s
4 407 tests ±0  4 320 ✅ ±0  87 💤 ±0  0 ❌ ±0 
4 416 runs  ±0  4 329 ✅ ±0  87 💤 ±0  0 ❌ ±0 

Results for commit 690a12b. ± Comparison against base commit 27d0b67.

Copy link
Contributor

@papeh papeh left a comment

Choose a reason for hiding this comment

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

Looks good overall.

/// </summary>
/// <param name="argument">the hvo of the record</param>
/// <returns></returns>
bool JumpToRecord(object argument, bool popup = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment about the purpose of popup would be helpful

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The popup parameter was deleted because it is no longer being used. It used to prevent the filter being cleared, but that was a mistake.

Copy link
Contributor

Choose a reason for hiding this comment

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

oops, sorry! I was looking at your PR backwards and thought the param was being added. 🤦‍♂️

/// </summary>
/// <param name="argument">the hvo of the record</param>
/// <returns></returns>
bool JumpToRecord(object argument, bool popup = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

oops, sorry! I was looking at your PR backwards and thought the param was being added. 🤦‍♂️

@jtmaxwell3 jtmaxwell3 merged commit 5c9637b into main Mar 10, 2026
6 checks passed
@jtmaxwell3 jtmaxwell3 deleted the LT-22450 branch March 10, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants