unfoldingWord / gateway-edit

Book Package harmonized view.
https://gatewayedit.com
MIT License
1 stars 4 forks source link

Scripture is not showing reference ranges #601

Closed kintsoogi closed 3 months ago

kintsoogi commented 9 months ago

Scripture reference ranges are shown when translation note with range is the first element on the list when the page is loaded. However, when a range is added or navigated to by clicking the arrows, the reference range is shown for a quick second and then scripture just shows a single verse.

DoD

Scripture shows reference range when new note is added and when reference range is navigated to when not first translation note.

kintsoogi commented 9 months ago

This is not a problem when I am in malachi, but it is a problem when I'm in Habakkuk

For some reason the versesForRef use effect is being called twice in Habakkuk, but only once in Malachi.

kintsoogi commented 9 months ago

This is a tricky bug....

It is caused by a race condition where:

  1. Reference was being set to null when the TSV note changed. This was set to null because the code assumed an empty string to be a non-valid value for the Quote field.
  2. Reference was being correctly set to the reference range when navigating to a reference range note

Since I didn't want to change the existing functionality of setItem() in translation notes since I was unsure how that would affect the application, I chose to solve the problem by having setItemto only pass in a null value to setCurrentCheck when the needed values were not strings. If the values are not present in the item array, the value of these parameters it is checking for would be undefined. Only in this case should we pass a null value to setCurrentCheck. Empty string values are totally valid.

kintsoogi commented 9 months ago

@elsylambert @danielklapp This just got merged into develop for 2.3 Develop SHA 548dd

Describe what your pull request addresses

Test Instructions

elsylambert commented 8 months ago

Verified in v2.3.0 build bd4a646 QA. Scripture pane shows the reference ranges when a tN has a reference range in it.

Screenshot 2023-12-19 at 9.47.20 AM.png
danielklapp commented 8 months ago

Looks good to me. Tested with v2.3.0 build bd4a646 QA.