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.

Sign up for GitHub

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

Jump to bottom

Support opening video timestamps in a new window #4687

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Support opening video timestamps in a new window #4687

merged 2 commits into from
Feb 26, 2024

Conversation

absidue
Copy link
Member

@absidue absidue commented Feb 18, 2024

Support opening video timestamps in a new window

Pull Request Type

  • Feature Implementation

Related issue

closes #4018

Description

Screenshots

context-menu-for-timestamp-link

Testing

  1. Find a video with timestamps in the description e.g. https://youtu.be/Rb0k9vUCEno
  2. Clicking on the timestamps normally should still seek the currently open video (regression test)
  3. Middle clicking (e.g. on the scroll wheel) and using the context menu on a timestamp, should open the current video in a new window at that timestamp.
  4. You can also test that copying the YouTube and Invidious links works as expected

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.19.1

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) February 18, 2024 10:22
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Feb 18, 2024
efb4f5ff-1298-471a-8973-3d47447115dc
efb4f5ff-1298-471a-8973-3d47447115dc reviewed Feb 18, 2024
View reviewed changes
Copy link
Member

Choose a reason for hiding this comment

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

IV link didnt go to the right timestamp for me?

VirtualBoxVM_HvOoGf0qvo.mp4

@PikachuEXE
Copy link
Collaborator

Local API:

  • left clicking on timestamp does not scroll view to top (w/ this PR)
  • right click & middle click seem fine

IV API:

@absidue
Copy link
Member Author

absidue commented Feb 19, 2024 •
edited
Loading

IV link didnt go to the right timestamp for me?

VirtualBoxVM_HvOoGf0qvo.mp4

That's definitely not caused by this pull request as it doesn't go anywhere near any of the code responsible for that.

The issue is with the Copy Invidious Link context menu code in the main process (notice how the Invidious URL gets two ? instead of one ? and one &).

@absidue
Copy link
Member Author

absidue commented Feb 19, 2024

The Invidious problem is that the URL contains two ?, the bug is in the context menu code in the main (src/main/index.js) process. We probably also need to update the code so it hides the copy link options for user playlists.

@PikachuEXE PikachuEXE mentioned this pull request Feb 19, 2024
Merged
4 tasks
@PikachuEXE
Copy link
Collaborator

Created fix for that in #4690

@PikachuEXE
Copy link
Collaborator

left clicking on timestamp does not scroll view to top (w/ this PR)

This is due to the link now contains a URL instead of #
So maybe add window.scrollTo(0,0) to changeTimestamp

@absidue
Copy link
Member Author

absidue commented Feb 20, 2024

Added code to scroll to the top when a timestamp is clicked, to replicate the previous behaviour.

@PikachuEXE
Copy link
Collaborator

#4690 review is waiting for this to be merged :)

efb4f5ff-1298-471a-8973-3d47447115dc
efb4f5ff-1298-471a-8973-3d47447115dc approved these changes Feb 26, 2024
View reviewed changes
Copy link
Member

Choose a reason for hiding this comment

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

left clicking on timestamp does not scroll view to top (w/ this PR)

Should we maybe implement something similar when you click on a Chapter? To make the UX equal to this.

VirtualBoxVM_vuLDnHKk1w.mp4

@FreeTubeBot FreeTubeBot merged commit 259f7a6 into FreeTubeApp:development Feb 26, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Feb 26, 2024
@absidue absidue deleted the timestamp-links branch February 26, 2024 09:27
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Feb 27, 2024
@PikachuEXE
Merge branch 'development' into feature/playlist-search-videos-in-one…
37d0f5c 
…-user-playlist-2

* development:
  Translated using Weblate (Basque)
  Prevent layout shifts when thumbnails load in the chapter selector (FreeTubeApp#4713)
  Bump electron-builder from 24.9.1 to 24.12.0 (FreeTubeApp#4718)
  Bump sass-loader from 14.1.0 to 14.1.1 (FreeTubeApp#4719)
  Bump sass from 1.71.0 to 1.71.1 (FreeTubeApp#4716)
  Bump the eslint group with 2 updates (FreeTubeApp#4715)
  Support opening video timestamps in a new window (FreeTubeApp#4687)
  Translated using Weblate (Basque)
  Bump webpack-dev-server from 4.15.1 to 5.0.2 (FreeTubeApp#4695)
  Translated using Weblate (Japanese)
  Translated using Weblate (Czech)
  Translated using Weblate (Lithuanian)
  Translated using Weblate (Japanese)
  Translated using Weblate (Vietnamese)
  Bump youtubei.js to 9.1.0 for watch page fix (FreeTubeApp#4707)
  Translated using Weblate (Vietnamese)
  Translated using Weblate (Vietnamese)
  Translated using Weblate (Portuguese (Brazil))
@PikachuEXE PikachuEXE mentioned this pull request Feb 27, 2024
Merged
4 tasks
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Feb 27, 2024
@PikachuEXE
Merge branch 'feature/playlist-layout-shift' into custom-builds/tmp
e460710 
* feature/playlist-layout-shift:
  * Make video thumbnails have certain height before image loading starts to avoid layout shifts
  Translated using Weblate (Basque)
  Prevent layout shifts when thumbnails load in the chapter selector (FreeTubeApp#4713)
  Bump electron-builder from 24.9.1 to 24.12.0 (FreeTubeApp#4718)
  Bump sass-loader from 14.1.0 to 14.1.1 (FreeTubeApp#4719)
  Bump sass from 1.71.0 to 1.71.1 (FreeTubeApp#4716)
  Bump the eslint group with 2 updates (FreeTubeApp#4715)
  Support opening video timestamps in a new window (FreeTubeApp#4687)
  Translated using Weblate (Basque)
  Bump webpack-dev-server from 4.15.1 to 5.0.2 (FreeTubeApp#4695)
  Translated using Weblate (Japanese)
  Translated using Weblate (Czech)
  Translated using Weblate (Lithuanian)
  Translated using Weblate (Japanese)
  Translated using Weblate (Vietnamese)
  Bump youtubei.js to 9.1.0 for watch page fix (FreeTubeApp#4707)
  Translated using Weblate (Vietnamese)
  Translated using Weblate (Vietnamese)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChunkyProgrammer ChunkyProgrammer ChunkyProgrammer approved these changes

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc approved these changes

@PikachuEXE PikachuEXE PikachuEXE approved these changes

@kommunarr kommunarr Awaiting requested review from kommunarr

@MarmadileManteater MarmadileManteater Awaiting requested review from MarmadileManteater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@absidue @PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @ChunkyProgrammer @FreeTubeBot