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

Added swipe controls #238

Merged
merged 4 commits into from
Feb 10, 2018
Merged

Added swipe controls #238

merged 4 commits into from
Feb 10, 2018

Conversation

avently
Copy link
Contributor

@avently avently commented Feb 1, 2018

  • allowed to change brightness and volume via vertical swipe
  • seek video is supported too via horizontal swipe
  • brightness is saved across restarts in sharedPreferences and will be applied automatically on video load
  • cool vector icons for all screens

Found time and made a normal PR with less code than before and adapted to current code base.

- allowed to change brightness and volume via vertical swipe
- seek video is supported too via horizontal swipe
- brighntess is saved across restarts in sharedPreferences and will be applied automatically on video load
- cool vector icons for all screens
@ram-on
Copy link
Collaborator

ram-on commented Feb 1, 2018

Hi @avently, I'll test it out this weekend and I'll get back to you.

@ildar
Copy link

ildar commented Feb 1, 2018 via email

@avently
Copy link
Contributor Author

avently commented Feb 1, 2018

@ildar you mean you want to skip more than a minute via one swipe? For example, first 10% of the screen width will skip 20 seconds, 20% will skip 1 minute, then 2 minutes, 5 minutes, etc?

@atomjack
Copy link

atomjack commented Feb 1, 2018

Well done, @avently !! Looks great!!

@ildar
Copy link

ildar commented Feb 1, 2018 via email

- seek speed will increase when you'll make swipe longer
@avently
Copy link
Contributor Author

avently commented Feb 2, 2018

@ildar now it's a little bit cooler

@ildar
Copy link

ildar commented Feb 2, 2018 via email

@ram-on
Copy link
Collaborator

ram-on commented Feb 4, 2018

Hello @avently, thanks for your implementation - looks great. The following is my feedback:

  1. Methods inside OnSwipeTouchListener that do not have any code should be changed to abstract.
  2. OnSwipeTouchListener#viewRect() should be changed to private and to abstract. Also add comments on what it does.
  3. What does onGestureDone() do? Also the parameter notStart is not being used - so we should remove that.
  4. Add (javadoc) comments (i.e. /** ... */) to each method and explain what each parameter does.
  5. For adjustBrightness() and adjustVolumeLevel() inside YouTubePlayerFragment: Pls add comments describing the major actions taking place.
  6. YouTubePlayerFragment#formatDuration(): This can be changed to private and pls remove the static keyword.

@avently
Copy link
Contributor Author

avently commented Feb 4, 2018

Hello @ram-on
OnSwipeTouchListener#viewRect() is important. It can't be changed to private static. It returns actual frame of the view that overrides this method. I can't get actual rect from touchListener without it.

Other is done.

@ram-on
Copy link
Collaborator

ram-on commented Feb 10, 2018

Looks good. Thanks

@ram-on ram-on merged commit 22fa500 into SkyTubeTeam:master Feb 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants