paranext / paranext-core

Electron client, extension host, and C# library for Paranext
https://paranext.github.io/paranext-core/
MIT License
16 stars 2 forks source link

Hook up Scroll Group UI to make it work in the editor #940

Open lyonsil opened 2 months ago

lyonsil commented 2 months ago

User Story As a user, I want to have different tabs scroll together at different references so that I can view different texts at different places in Scripture for things like Parallel Passages or cross references.

Description Implement the functionality side following the UX Scroll Group Design at https://paratext.myjetbrains.com/youtrack/issue/BUX-17.

Make this work with the editor as a starting point. It is assumed that the mechanism determined would work with other extensions.

Making scroll groups work with other applications running in the OS at the same time is out of scope for this ticket.

One possible implementation:

irahopkinson commented 2 months ago

I don't have access to BUX-17 (no YouTrack account - this is an issue for several of us) so I can't see what I need to respond well, but here are some questions around the implementation:

  1. Presumably each scroll group has its own BCV value that is passed around?
  2. Do we scroll to about a third of the way down the viewport so we have some context above but most context below?
  3. Should scrolling happen in the Editor or in P.B (who needs the control over scrolling)? 3.1 Context: The Editor already updates the BCV value when the cursor moves and moves the cursor to the beginning of the verse when the BCV value is updated. P.B currently has some code to scroll that was added for UX testing but it's buggy. That code also momentarily highlights verse numbers when the cursor moves but that functionality is out of scope of this issue.
tjcouch-sil commented 2 months ago

Ira, regarding your question 3, I imagine you probably have much more context to work with inside the editor such that you could make a better, less-hacky version of the scrolling than I did. Would be great if you could investigate sometime what it would look like to implement the scrolling inside the editor (maybe with a prop to enable/disable) and we could compare.

Regarding your question 2, I believe UX wanted me to try to scroll so 3 lines above the current focus were visible for the UX testing they did. I was too lazy to get the literal line height in pixels in order to do the scrolling, though, so I faked it haha

tjcouch-sil commented 2 months ago

Another note: I think it would be especially helpful to have this kind of scrolling (or at least verse click handler) functionality in the editor itself so other contexts can have this functionality without having to rewrite it themselves. Like in the text collection, we don't have scrolling or verse selecting hooked up because all that code is only in the editor web view even though the text collection web view is using the editor component.

ianhewubs commented 1 month ago

It might be worth mentioning: when a pane gets focus, before updating the scroll location of all other panes, first check whether the current verse is already in view in all panes. For a while we had really annoying behaviour in Paratext where one or two panes would needlessly update their scroll location as a user clicked between panes.

tjcouch-sil commented 3 weeks ago

This might be a natural time to hook up the editor selection into web view state

tjcouch-sil commented 1 day ago

Reworking this and #788:

tjcouch-sil commented 1 day ago

Example of tracking which iframe has focus https://jsfiddle.net/9285tbsm/9/ unfortunately it doesn't work if you click one iframe then the other. Only works if you click top again. Not a great solution