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

Add focus range #94

Merged
merged 61 commits into from Mar 25, 2022
Merged

Add focus range #94

merged 61 commits into from Mar 25, 2022

Conversation

fulopkovacs
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Mar 4, 2022

THE-110 Focus Range

Description

A way to temporarily focus playback to a certain range

We had this in the older theatre

0d51f4f7-6b30-4ebd-b3f7-b223bd0943cc

Hints

  • Ask Aria for how to save the focus' state on the store
  • Ask Aria for how to read the state reactively (hint: use usePrism())
  • Use the existing context menu hook useContextMenu()
  • Find out where in the code, the spacebar key is read. You probably have to read it in your own component, so you can capture those keypresses

@fulopkovacs fulopkovacs added enhancement studio Related to @theatre/studio and removed enhancement labels Mar 4, 2022
@fulopkovacs
Copy link
Contributor Author

Status: WIP, not ready to merge!

I have reached the point where we have a messy Focus Range with dragable and movable handles for the start and end points of the range. It also controls the playback once you moved the handles AND restarted the playback. I have some thoughts on how to proceed, but a review now would save us a few other ones in the future. 😆 A Slack-huddle/Zoom call would be appreciated, since I have a few questions.

@AriaMinaei
Copy link
Member

Looks like the drag gesture has a drift in FF depending on zoom level, but not in Chrome.

Screenflick.Movie.68.mp4

fulopkovacs and others added 21 commits March 18, 2022 15:59
…playhead is outside of the focusRange when the playback is started
@netlify
Copy link

netlify bot commented Mar 18, 2022

Deploy Preview for theatrejs-playground failed.

Name Link
🔨 Latest commit 6bafdf6
🔍 Latest deploy log https://app.netlify.com/sites/theatrejs-playground/deploys/623de2aac3718f0009b9f23d

@fulopkovacs
Copy link
Contributor Author

fulopkovacs commented Mar 22, 2022

Current state of the PR:

  • fix the height of the highlighted part of the DopeSheet
  • minimal size
  • double click for max size
  • replace the current color of focusRangeStrip when dragging
  • replace the current color of focusRangeThumb when dragging
  • make the playhead bigger when the focusRange is used for the playback
  • [bugfix] Don't highlight the range on hover when it's disabled
  • [feature] display tooltips above the focusRangeThumb-s when dragging
  • [bugfix] fix the visuals of the playhead (it looks bad when it's scaled up)
  • [refactor] remove the "double click to expand" feature (Mariusz approved this)
  • [refactor] refactor the focusRangeTheme object
  • [refactor] review and clean up the PR

Copy link
Member

@AriaMinaei AriaMinaei left a comment

Choose a reason for hiding this comment

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

This is great! Just a couple suggestions:

  1. Review the code comments and comment anything that's not obvious (like sequence.playInDynamicRange()
  2. Make the thumbs snappable.

@fulopkovacs
Copy link
Contributor Author

fulopkovacs commented Mar 24, 2022

Current status after the 1st review:

  • Make the thumbs snapable
  • Add comments to the parts that are harder to read

@AriaMinaei AriaMinaei merged commit ffdebeb into main Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
studio Related to @theatre/studio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants