unfoldingWord / scripture-resources-rcl

A React Component Library for Rendering Scripture Resources.
https://scripture-resources-rcl.netlify.com/
MIT License
0 stars 8 forks source link

It should not be assumed that the Original Language resource is always first in ScriptureTable books array param. #174

Closed abelpz closed 1 year ago

abelpz commented 1 year ago

Summary

ScriptureTable has a param called books, this is being passed in to it by ParallelScripture component after processing an array of resource URLs, this array is supposed to contain URLs for: an Original Language (Source), translations aligned to the previous original language. The problem is the Original Language is assumed to always be first in that array and ScriptureTable renders each resource in the same order they are set in that resources array, so when users want to move the original languages to a different position, some features like highlighting break.

Critical places where hardcoded position is breaking highlighting:

https://github.com/unfoldingWord/scripture-resources-rcl/blob/138664f1a2ae8a9c784f5dad9742722f36a29685/src/components/parallel-scripture/ScriptureTable.js#L148C26-L148C29

https://github.com/unfoldingWord/scripture-resources-rcl/blob/138664f1a2ae8a9c784f5dad9742722f36a29685/src/components/parallel-scripture/helpers.js#L92C11-L92C22

The Zero (0) in both places refers to the position of the Original Language resource.

Motivation

DoD

Changing the order of the resources array sent to the ParallelScripture component should not break alignment.

Joel-C-Johnson commented 1 year ago

thanks for creating the issue with proper details. I will work on this.

Joel-C-Johnson commented 1 year ago

Solved the issue: https://github.com/unfoldingWord/tc-create-app/issues/1600#issuecomment-1695610287

Joel-C-Johnson commented 1 year ago

For testing:

https://deploy-preview-173--scripture-resources-rcl.netlify.app/

Joel-C-Johnson commented 1 year ago

Test Instruction in RCL :

Note: In defaultResourceLinks the value unfoldingWord/${path.nt}/master/${bookId} is the original (Greek / Hebrew) language, So if we move this link to a different index and also change the value of ORIGINAL_LANG_POSITION as the same index value.

danielklapp commented 1 year ago

Looks good to me. User can place the Original Language in any position they prefer in the array without breaking alignment. Tested with ScriptureResourcesRcl v5.5.5.

Joel-C-Johnson commented 1 year ago

Looks good to me. User can place the Original Language in any position they prefer in the array without breaking alignment. Tested with ScriptureResourcesRcl v5.5.5.

Moving to QA Done as it is already merged and closed.