Fix LT-22539: Loss of focus from gloss field#931
Open
jtmaxwell3 wants to merge 8 commits into
Open
Conversation
NUnit Tests 1 files ± 0 1 suites ±0 10m 14s ⏱️ - 1m 34s Results for commit 8f391e6. ± Comparison against base commit 53d5dbb. This pull request removes 2 and adds 43 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Render comparison artifactsRender snapshot failures were reported in 3912e756d998 run 27293398351.1, but the latest run 6bd317098f96 run 27423253555.1 is clean. This comment will be replaced if a future run produces render snapshot failures again. |
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes interlinear Focus Box refresh behavior so edits aren’t lost and focus isn’t stolen when related lexicon/MSA data changes (LT-22539 / LT-22534 / LT-22541). The change replaces a “reselect + move focus box” refresh with a targeted field update that aims to preserve the secondary cache and user focus.
Changes:
- Route
InterlinDocForAnalysis.PropChangedrefreshes throughFocusBoxController.UpdateFieldinstead of reselecting the occurrence and repositioning the focus box. - Add
SandboxBase.UpdateFieldto refresh secondary-cache sense/MSA display viaEstablishDefaultSense. - Add plumbing (
IAnalysisControlInternal.UpdateField) and update test doubles/designer usings accordingly.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Src/LexText/Interlinear/SandboxBase.cs | Adds secondary-cache refresh helper (UpdateField) and a morph lookup utility to update displayed sense/MSA without clearing caches. |
| Src/LexText/Interlinear/ITextDllTests/InterlinDocForAnalysisTests.cs | Updates mock sandbox to satisfy new interface method. |
| Src/LexText/Interlinear/InterlinDocForAnalysis.Designer.cs | Adds (currently unused) using directive. |
| Src/LexText/Interlinear/InterlinDocForAnalysis.cs | Switches PropChanged refresh from “reselect/move” to FocusBox.UpdateField. |
| Src/LexText/Interlinear/FocusBoxController.cs | Adds focus-suppression flag and UpdateField forwarding; extends internal sandbox interface. |
Files not reviewed (1)
- Src/LexText/Interlinear/InterlinDocForAnalysis.Designer.cs: Language not supported
Comment on lines
+1659
to
+1661
| public void UpdateField(int hvo, int flid) | ||
| { | ||
| ICmObject hvoObject = Caches.MainCache.ServiceLocator.GetInstance<ICmObjectRepository>().GetObject(hvo); |
Comment on lines
+1670
to
+1671
| if (hvoObject != null && hvoObject.Owner is IMoMorphSynAnalysis msa) | ||
| { |
Comment on lines
+318
to
+330
| public void UpdateField(int hvo, int flid) | ||
| { | ||
| try | ||
| { | ||
| // This fixes LT-22539. | ||
| suppressFocusChange = true; | ||
| m_sandbox.UpdateField(hvo, flid); | ||
| } | ||
| finally | ||
| { | ||
| suppressFocusChange = false; | ||
| } | ||
| } |
| get { return m_sandbox.IsDirty; } | ||
| } | ||
|
|
||
| private bool suppressFocusChange = false; |
| // This software is licensed under the LGPL, version 2.1 or later | ||
| // (http://www.gnu.org/licenses/lgpl-2.1.html) | ||
|
|
||
| using SIL.FieldWorks.Common.FwUtils; |
Comment on lines
+561
to
+564
| public void UpdateField(int hvo, int flid) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } |
Comment on lines
+1659
to
+1663
| public void UpdateField(int hvo, int flid) | ||
| { | ||
| ICmObject hvoObject = Caches.MainCache.ServiceLocator.GetInstance<ICmObjectRepository>().GetObject(hvo); | ||
| if (hvoObject is ILexSense lexSense && lexSense.Owner is ILexEntry lexEntry) | ||
| { |
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
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.
This fixes https://jira.sil.org/browse/LT-22539, https://jira.sil.org/browse/LT-22534, and https://jira.sil.org/browse/LT-22541. I discovered that MoveFocusBoxIntoPlace caused the secondary cache to be cleared, which meant that any user edits were lost. So I replaced it with UpdateField and EstablishDefaultSense. EstablishDefaultSense is an existing function which updates the sense name and msa in the secondary cache and the display. FocusBoxController.UpdateField suppresses the focus change to fix LT-22539. SandboxBase.UpdateField fixes LT-22534 and LT-22541. I put all of these fixes in the same commit to make sure that they all worked together.
This change is