abdelaziz-mahdy / flutter_meedu_videoplayer

Cross-Platform Video Player for flutter
https://abdelaziz-mahdy.github.io/flutter_meedu_videoplayer/
MIT License
132 stars 69 forks source link

added: slider color now relies on theme #105

Closed aleeeee1 closed 1 year ago

aleeeee1 commented 1 year ago

tested only in dark mode.

abdelaziz-mahdy commented 1 year ago

Will check dark and light mode to make both look good , then merge hopefully,

Thank you for your contribution ❤️

abdelaziz-mahdy commented 1 year ago

just checked the look, sadly i dont think i cant merge this in the current look, since its not looking good in my end

image image
aleeeee1 commented 1 year ago

yeah, no worries, i'm trying to make pip work in ios too. if i manage to do that, i'll make a new pull where it will actually look good. :D

abdelaziz-mahdy commented 1 year ago

That would be awesome if you are able to do iOS pip, looking forward for the PR

aleeeee1 commented 1 year ago

i managed to make it work, but it's not stable nor a good implementation. this library code has been barely been touched, just implemented a method on ios channel for checking pip availability and then the methods for checking/starting, very minimal changes.

i tried implementing it manually but it's a mess, i'm not that good, then i saw a pr on video_player that is waiting for approval since 3months, which implements pip for ios.

https://github.com/vanlooverenkoen/flutter-packages/tree/feature/%2360048-ios-picture-in-picture

i coudln't manage to implement the import directly in this library, so I made a dependency override in my app's pubspec.yaml

dependency_overrides:
  video_player_avfoundation:
    git:
      url: https://github.com/vanlooverenkoen/flutter-packages/
      ref: feature/#60048-ios-picture-in-picture
      path: packages/video_player/video_player_avfoundation

  video_player_platform_interface:
    git:
      url: https://github.com/vanlooverenkoen/flutter-packages/
      ref: feature/#60048-ios-picture-in-picture
      path: packages/video_player/video_player_platform_interface

  video_player:
    git:
      url: https://github.com/vanlooverenkoen/flutter-packages/
      ref: feature/#60048-ios-picture-in-picture
      path: packages/video_player/video_player

it's a bare workaround, i know, but this is the best i came up with. we'll wait for video_player to accept the pr i guess, until then idk. I'll check out the colors for the sliders and then i'll push another pr, which you shouldn't approve since it's broken code unless you don't override the dependencies, but may be useful for someone who needs it like me. maybe i could do two branches, one with the slider colors and one with the "ios pip implementation" with a brief tutorial on how to achive that

abdelaziz-mahdy commented 1 year ago

i managed to make it work, but it's not stable nor a good implementation. this library code has been barely been touched, just implemented a method on ios channel for checking pip availability and then the methods for checking/starting, very minimal changes.

i tried implementing it manually but it's a mess, i'm not that good, then i saw a pr on video_player that is waiting for approval since 3months, which implements pip for ios.

https://github.com/vanlooverenkoen/flutter-packages/tree/feature/%2360048-ios-picture-in-picture

i coudln't manage to implement the import directly in this library, so I made a dependency override in my app's pubspec.yaml

dependency_overrides:
  video_player_avfoundation:
    git:
      url: https://github.com/vanlooverenkoen/flutter-packages/
      ref: feature/#60048-ios-picture-in-picture
      path: packages/video_player/video_player_avfoundation

  video_player_platform_interface:
    git:
      url: https://github.com/vanlooverenkoen/flutter-packages/
      ref: feature/#60048-ios-picture-in-picture
      path: packages/video_player/video_player_platform_interface

  video_player:
    git:
      url: https://github.com/vanlooverenkoen/flutter-packages/
      ref: feature/#60048-ios-picture-in-picture
      path: packages/video_player/video_player

it's a bare workaround, i know, but this is the best i came up with. we'll wait for video_player to accept the pr i guess, until then idk. I'll check out the colors for the sliders and then i'll push another pr, which you shouldn't approve since it's broken code unless you don't override the dependencies, but may be useful for someone who needs it like me. maybe i could do two branches, one with the slider colors and one with the "ios pip implementation" with a brief tutorial on how to achive that

Yes another pr to help people would be awesome, but keep in mind this workaround may not work on media_kit for iOS , right now the pip for Android works on both video_player and media_kit by making the app go to pip , not the player alone

Anyway thanks for your work, will check the pr when you create it and check if I can implement it in this package