Closed cparish312 closed 1 month ago
Hey Connor! Congrats on your first PR! Excited to review this when I get a chance.
Could you please include a short video of going full screen with the news approach?
Does it not occlude the menu bar of the recording, does it fill the whole display, and does it not play the "full screening" transition / animation? These were the "requirements" I was attempting to operate under.
Hey Jason! On my end it seems to have almost same behavior as the previous fullscreening method minus the external monitor blacking out. One thing I noticed is there is now an animation showing a scrolling to the screen. This is due to the change from .fullScreenAuxillary to .fullScreenPrimary. Not sure if that counts as ""full screening" transition / animation".
Finding my first stabs at UI work to be.... educational
Just tested this - yeah the new method incurs a pretty significant delay opening the timeline due to the full screen transition. Though it's faster on subsequent opens.
Old Method:
https://github.com/jasonjmcghee/rem/assets/1522149/4c61e865-71ed-4f01-9165-920376f5e3a8
New Method:
https://github.com/jasonjmcghee/rem/assets/1522149/025edcdb-ff9a-4d13-8b0d-be3da4b9563f
If nothing else, we could allow toggling between these approaches but I'm hoping there's a happy medium that fixes the issues you identified without needing the full screen window delays.
Making an image / video full screen with preview doesn't have this problem... 🤔
After many iterations, I ended up with the first thing I tried (but this time a working version) 😂
https://github.com/jasonjmcghee/rem/assets/54302513/290ab2b8-cd83-476e-8264-4d35bdbbf7fd
🎉 Congrats on joining as a rem contributer! 🎉
Previously the method of having the timeline view go fullscreen caused external monitors to blackout (and just kinda freakout generally)
I tried to also add in seeing the mac menubar when the user scrolls up but instead an option to exit full screen is given.
https://github.com/jasonjmcghee/rem/assets/54302513/f29a2e8e-fabf-4520-935b-b50c60feb36b