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 Duck Player settings UI #4748

Conversation

CrisBarreiro
Copy link
Contributor

@CrisBarreiro CrisBarreiro commented Jul 10, 2024

Task/Issue URL: https://app.asana.com/0/72649045549333/1207551634140420

Description

  • Add Duck Player settings UI
  • Open YT URLs in Duck Player if setting is Enabled(Always)

Steps to test this PR

https://app.asana.com/0/0/1207704461779423/f
Note: There have been changes to the JS integration, so you might need to test these changes on the top of the stack

User preferences "Always" open Duck Player

  • Open Settings -> Duck Player -> Set to always
  • Type a YT URL on the omnibar
  • Check Duck Player is loaded
  • Navigate back and check you're going to the previous page your were visiting

User preferences "Always ask" trigger overlay

  • Open Settings -> Duck Player -> Set to always ask
  • Type a YT URL on the omnibar
  • Check overlay is loaded in YT
  • Check that watch here removes the overlay, and watch in Duck player navigates to Duck Player

User preferences "Never" stays in YT

  • Open Settings -> Duck Player -> Set to never
  • Type a YT URL on the omnibar
  • Check video is loaded normally

UI changes

See https://app.asana.com/app/asana/-/get_asset?asset_id=1207785858877769

Copy link
Contributor Author

CrisBarreiro commented Jul 10, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @CrisBarreiro and the rest of your teammates on Graphite Graphite

@CrisBarreiro CrisBarreiro mentioned this pull request Jul 10, 2024
18 tasks
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch 3 times, most recently from 32a00b5 to 95f055e Compare July 11, 2024 07:52
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from 40ab3e7 to 0a92af2 Compare July 11, 2024 15:43
@CrisBarreiro CrisBarreiro marked this pull request as ready for review July 11, 2024 15:48
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from 0a92af2 to 1e20bc1 Compare July 11, 2024 15:48
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from 1e20bc1 to d3483e0 Compare July 11, 2024 15:51
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/create-module branch from 4981aed to c8e7b54 Compare July 19, 2024 11:35
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from d3483e0 to 4a6069c Compare July 19, 2024 11:36
@CrisBarreiro CrisBarreiro mentioned this pull request Jul 31, 2024
3 tasks
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/create-module branch from c8e7b54 to 01cdb4e Compare August 7, 2024 09:56
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from 4a6069c to 1c70aab Compare August 7, 2024 09:56
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/create-module branch from 01cdb4e to 7d9aa0a Compare August 7, 2024 10:28
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from 1c70aab to f27c311 Compare August 7, 2024 10:28
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/duckplayer/native-settings branch from ef92132 to 4330616 Compare August 8, 2024 11:50
Copy link
Contributor

@marcosholgado marcosholgado left a comment

Choose a reason for hiding this comment

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

On the PR the settings don't seem to change but at the top of the stack works. Code wise LGTM though I think we should add more tests :)

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.

3 participants