damus-io / damus

iOS nostr client
GNU General Public License v3.0
1.96k stars 289 forks source link

Bug: Portrait video cropping on full screen carousel view looks bad #2165

Open danieldaquino opened 3 months ago

danieldaquino commented 3 months ago

What happens

On the full screen carousel view, portrait videos look small, because they are set to "fit" the video player, not "fill"

What I expect to happen

Portrait videos should be cropped differently on that view. Preferably the sides of the video should touch the sides of the screen.

Link to noteID, npub Example note: note1f2uwa5ep9c79er4p7nz993jeaa6zmn6sdjeuehay90q03dj0vnqsztplhr

Screenshots/video recording IMG_22E3D4C76728-1

Versions Damus version: 1.9 (1) Operating system version: iOS 17.3.1 Device: iPhone 13 Mini

Steps To Reproduce

  1. Open Damus
  2. Go to a note with a portrait video (like the one linked)
  3. Click on the video to open the full screen carousel view.
  4. See how the video looks like

Additional context The current view works well with landscape videos. If increasing the height of the video player, we should be careful not to make the controls hidden behind the event text, and/or make it so big that we cannot tap outside of it to hide the event text.

jb55 commented 3 months ago

yeah this is was referring to in our last standup

danieldaquino commented 3 months ago

yeah this is was referring to in our last standup

Yep! I am dumping my notes and stuff I saw on emails or conversations into Github issues to make sure we don't lose track of them

jb55 commented 3 months ago

On Wed, Apr 17, 2024 at 01:02:39PM -0700, Daniel D’Aquino wrote:

yeah this is was referring to in our last standup

Yep! I am dumping my notes and stuff I saw on emails or conversations into Github issues to make sure we don't lose track of them

I figured! just wanted to make sure.