Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SF-2658 Audio player randomly scrolls back to active question #2480

Merged
merged 1 commit into from
May 28, 2024

Conversation

nigel-wells
Copy link
Collaborator

@nigel-wells nigel-wells commented May 24, 2024

  • Stop highlighting and scrolling when setting an active verse to the same value

The underlaying cause is when a remote user makes a change to the question doc query it triggers an update for all users subscribed to that query. This change triggers the active verse to get set again which then highlights and scrolls to the verse being set.

Acceptance tests

  • Configure a project for community checking
  • Load in questions and attach chapter audio and timing data
  • Grant 2 users access to community checking on the project

Browser 1

  1. Select the first question
  2. Start playing chapter audio
  3. Wait for the highlighted verse change away from the verse of the selected question

Browser 2

  1. Select the first question
  2. Interact in some way i.e. answer the question, like an answer, etc.

Observe
When a remote browser is interacting with questions it shouldn't affect the other browser whose active question isn't changing.


This change is Reviewable

@nigel-wells nigel-wells added the will require testing PR should not be merged until testers confirm testing is complete label May 24, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.51%. Comparing base (c0b48f5) to head (0aba4a7).

Current head 0aba4a7 differs from pull request most recent head efa7c47

Please upload reports for the commit efa7c47 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2480      +/-   ##
==========================================
+ Coverage   77.49%   77.51%   +0.02%     
==========================================
  Files         510      510              
  Lines       29161    29235      +74     
  Branches     4737     4766      +29     
==========================================
+ Hits        22598    22662      +64     
+ Misses       5824     5818       -6     
- Partials      739      755      +16     

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

@pmachapman pmachapman self-assigned this May 26, 2024
@pmachapman pmachapman self-requested a review May 26, 2024 20:37
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:

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

@pmachapman pmachapman added ready to test and removed will require testing PR should not be merged until testers confirm testing is complete labels May 26, 2024
@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 May 28, 2024
* Stop highlighting and scrolling when setting an active verse to the same value
@Nateowami Nateowami force-pushed the fix/SF-2658-active-verse branch from 0aba4a7 to efa7c47 Compare May 28, 2024 14:38
@Nateowami Nateowami enabled auto-merge (squash) May 28, 2024 14:38
@Nateowami Nateowami merged commit 728587d into master May 28, 2024
14 checks passed
@Nateowami Nateowami deleted the fix/SF-2658-active-verse branch May 28, 2024 14:46
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.

4 participants