8000 fix: Scroll issues in rundowns by olzzon · Pull Request #1420 · nrkno/sofie-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Scroll issues in rundowns #1420

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 38 commits into
base: release53
Choose a base branch
from
Open

fix: Scroll issues in rundowns #1420

wants to merge 38 commits into from

Conversation

olzzon
Copy link
Contributor
@olzzon olzzon commented Apr 8, 2025

About the Contributor

This PR is made on behalf of BBC

Type of Contribution

This is a bug fix + refactor

Current Behavior

  • Minishelf is not part of the calculated size of the rundown, so when next, it scrolls to far and detach from on-air line
  • Lot's of screenflickering when loading a rundown
  • Performance issues on take as over 10 updates of on-air components when next.
  • On-Air button in righthand menu, was disappearing if scrolling immediately after pressing it, and a reload of page was necessary before connecting to on-air line
  • Scrolling was not smooth

New Behavior

  • detach from on-air line has a delay before it is triggered
  • refactoring of segment scrolling, measuring and position

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

Affected areas

This affects rundown view, and as it's a refactor, there can be unknown edge cases.
It has been tested with rundowns from BBC and NRK

Time Frame

This should be merged prior to the planned refactoring of the rundown view.

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

olzzon added 30 commits March 18, 2025 15:50
10000
@olzzon
fix: let UI settle before testing for segment being in view
83c38f1
@olzzon
fix: lower time for UI settle before changing isVisible
a4fd32d
@olzzon
fix: recursive event emits to onGoToPartInstance when scrollToPartIns…
f47b911 
…tance was called

fix, use timeout and measure scroll to position
@olzzon
fox: remove requestIdleCallback implementation as scrollToPosition is…
de7f162 
… no longer called 10 times on a take
@olzzon
fix: maintainFocusOnPartInstance race condition
cb7fcb1
@olzzon
fix: remove left over console.logs
920a30e
@olzzon
fix: virtualElement shouldn't adjust while scrolling. Earlier there w…
c4dc586 
…as just a 5sec delay to adjust the virtualElement. Instead there's a state in viewPort telling if it's scrolling
@olzzon
feat: lower delay before scrollstart
e47db49
@olzzon
feat: add message for 2.stage scroll to log how often this happens. (…
c0252b0 
…it shouldn't happen at all)
@olzzon
fix: raise wait before scroll to ensure element is ready
d870be5
@olzzon
fix: to slow update if segment need 2.stage adjustment
85f347d
@olzzon
fix: ensure 2.stage is not ran until virtualelement has been updated …
143e3f3 
…(fix if more than 1 segment is invalid)
@olzzon
fix: correct height of VirtualElements
0fd7e67
@olzzon
fix: cleanup pendingFirstStagetimeout
eca15b0
@olzzon
fix: don't expose viewPortScrollingState use getViewPortScrollingStat…
26cc415 
…e() instead
@olzzon
fix: use throttle in onWheelScrollInner for more fluid scrolling
5bb269f
@olzzon
fix: Safari race condition in virtualElement
d542aa9
@olzzon
fix: add delay on extra check when scrolling long list
aa92e49
@olzzon
feat: simplify VirtualElemt to avoid racecondition between useEffect …
6a8bbcc 
…and useLayoutEffect. When using the 'contain: 'size layout' option, a static placeholder is fine Chrome
@olzzon
fix: add precalculated measurement
408596e
@olzzon
feat: simplify size measure and inititaly use default height - prepar…
e497bda 
…e resize
@olzzon
feat: add resize support to virtual elements
32786bb
@olzzon
feat: prepare for dynamic resize observers based on inView
6a9a40e
@olzzon
feat: dynamic resize handler
8955053
@olzzon
feat: added singleton resize manager
307c8a7
@olzzon
fix: add mutation observer for resizes on activate and on take
a2272c3
@olzzon
fix: remote double measurement on load, as the observer takes care of…
ab2bb34 
… that now
@olzzon
fix: set isShowingChildren imidiatly when element are in view to avoi…
4a1679e 
…d timing issues
@olzzon
fix: add initial measurement of elements in view, to ensure correct s…
e385a97 
…ize after load
@olzzon
fix: timer active was lost - using useRef for timer reference
6fdee22
@olzzon olzzon requested a review from a team as a code owner April 8, 2025 08:30
@codecov Codecov
Copy link
codecov bot commented Apr 8, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.85%. Comparing base (d7ca0ed) to head (33b803e).
Report is 33 commits behind head on release53.

Additional details and impacted files 
@@              Coverage Diff              @@
##           release53    #1420      +/-   ##
=============================================
- Coverage      56.87%   56.85%   -0.02%     
=============================================
  Files            404      404              
  Lines          73147    73222      +75     
  Branches        4041     4457     +416     
=============================================
+ Hits           41603    41634      +31     
- Misses         31410    31447      +37     
- Partials         134      141       +7

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jstarpl jstarpl changed the title Fix: Scroll issues in rundowns fix: Scroll issues in rundowns Apr 8, 2025
@jstarpl jstarpl added Contribution External contribution Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) labels Apr 8, 2025
jstarpl
jstarpl requested changes Apr 24, 2025
View reviewed changes
Copy link
Member
@jstarpl jstarpl left a comment

Choose a reason for hiding this comment

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

Testing this locally, I see some issues with the new implementation of VirtualElement and it measuring the elements. Specifically, hidden Segments (isHidden) seem to be throwing it off.
obraz

I think there's also some issue with Segment placeholders and their margins (note the red line - the position of a regular Segment and the Placeholder don't align):
obraz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstarpl jstarpl jstarpl requested changes

Assignees
No one assigned
Labels
Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) Contribution External contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@olzzon @jstarpl
0