Skip to content

SF-3497 Fix no Lynx overlay when Lynx problems panel nav to another chapter#3374

Merged
Nateowami merged 2 commits intomasterfrom
fix/sf-3497-no-lynx-overlay-on-panel-nav-to-diff-chapter
Aug 20, 2025
Merged

SF-3497 Fix no Lynx overlay when Lynx problems panel nav to another chapter#3374
Nateowami merged 2 commits intomasterfrom
fix/sf-3497-no-lynx-overlay-on-panel-nav-to-diff-chapter

Conversation

@siltomato
Copy link
Collaborator

@siltomato siltomato commented Aug 15, 2025

This PR fixes an issue where the lynx overlay would not display when using the lynx problems panel to navigate to (and display the overlay for) an insight from a different chapter.

The fix involved waiting for the insights for the chapter to render before making the call to render the overlay.


This change is Reviewable

@siltomato siltomato added the will require testing PR should not be merged until testers confirm testing is complete label Aug 15, 2025
@codecov
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.21%. Comparing base (18309e7) to head (232ba24).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3374      +/-   ##
==========================================
+ Coverage   82.16%   82.21%   +0.04%     
==========================================
  Files         609      609              
  Lines       35895    35901       +6     
  Branches     5863     5840      -23     
==========================================
+ Hits        29493    29515      +22     
- Misses       5519     5524       +5     
+ Partials      883      862      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@siltomato siltomato force-pushed the fix/sf-3497-no-lynx-overlay-on-panel-nav-to-diff-chapter branch from 0f68a0d to 3c0703d Compare August 15, 2025 20:26
@siltomato siltomato marked this pull request as ready for review August 15, 2025 20:26
@pmachapman pmachapman self-assigned this Aug 17, 2025
@pmachapman pmachapman self-requested a review August 17, 2025 20:52
Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

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

:lgtm: Just one very minor nit in a .spec.ts file, so I will approve so to not hold up testing.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @siltomato)


src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-editor-objects/lynx-insight-editor-objects.component.spec.ts line 5 at r1 (raw file):

import { Delta } from 'quill';
import { BehaviorSubject } from 'rxjs';
import { TextDocId } from 'src/app/core/models/text-doc';

NIT: This should be:

import { TextDocId } from '../../../../../core/models/text-doc';

(I also noticed this in \src\SIL.XForge.Scripture\ClientApp\src\app\translate\editor\lynx\insights\lynx-insight-overlay\lynx-insight-overlay.component.spec.ts)

Code quote:

import { TextDocId } from 'src/app/core/models/text-doc';

@pmachapman pmachapman added ready to test and removed will require testing PR should not be merged until testers confirm testing is complete labels Aug 17, 2025
@siltomato siltomato force-pushed the fix/sf-3497-no-lynx-overlay-on-panel-nav-to-diff-chapter branch from 3c0703d to e427355 Compare August 18, 2025 15:26
Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @siltomato)

@Nateowami Nateowami added testing complete Testing of PR is complete and should no longer hold up merging of the PR and removed ready to test labels Aug 20, 2025
@Nateowami Nateowami changed the title SF-3497 Fix no lynx overlay when lynx problems panel nav to another chapter SF-3497 Fix no Lynx overlay when Lynx problems panel nav to another chapter Aug 20, 2025
@Nateowami Nateowami force-pushed the fix/sf-3497-no-lynx-overlay-on-panel-nav-to-diff-chapter branch from e427355 to 232ba24 Compare August 20, 2025 14:53
@Nateowami Nateowami enabled auto-merge (squash) August 20, 2025 14:53
@Nateowami Nateowami merged commit 0803616 into master Aug 20, 2025
15 of 16 checks passed
@Nateowami Nateowami deleted the fix/sf-3497-no-lynx-overlay-on-panel-nav-to-diff-chapter branch August 20, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing complete Testing of PR is complete and should no longer hold up merging of the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants