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

feat: YouTube player respects timestamps startAt #1620

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ggichure
Copy link
Collaborator

@ggichure ggichure commented Dec 3, 2024

Pull Request Description

  • Implement timestamp extraction for YouTube video URLs
  • Automatically set video start time from URL parameters
  • Support multiple timestamp formats (t=, start=, time notation)
  • Add custom method to parse timestamps in seconds

Issue Being Fixed

Issue Number: #1619

Screenshots / Recordings

thunder_yt_player_start_at.mp4

Checklist

  • If a new package was added, did you ensure it uses an appropriate license and is actively maintained?
  • Did you use localized strings (and added appropriate descriptions) where applicable?
  • Did you add semanticLabels where applicable for accessibility?

- Implement timestamp extraction for YouTube video URLs
- Automatically set video start time from URL parameters
- Support multiple timestamp formats (t=, start=, time notation)
- Add custom method to parse timestamps in seconds
Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

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

Hey thanks so much for taking on this issue and fixing it! I just left a few minor comments.

}

int _convertTimeComponentsToSeconds(RegExpMatch match) {
int hours = match.group(1) != null ? int.parse(match.group(1)!.replaceAll('h', '')) : 0;
Copy link
Member

Choose a reason for hiding this comment

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

You might be able to use RegEx groups so that one of the matches exactly matches the number without the unit (h, s, etc.).

int minutes = match.group(2) != null ? int.parse(match.group(2)!.replaceAll('m', '')) : 0;
int seconds = match.group(3) != null ? int.parse(match.group(3)!.replaceAll('s', '')) : 0;

return hours * 3600 + minutes * 60 + seconds;
Copy link
Member

Choose a reason for hiding this comment

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

You could do this as Duration(hours: hours, minutes: minutes, seconds: seconds).inSeconds to avoid "magic numbers".

}

int _convertTimeStringToSeconds(String time) {
final regex = RegExp(r'(\d+h)?(\d+m)?(\d+s)?');
Copy link
Member

Choose a reason for hiding this comment

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

Can you define this RegEx just once and reuse it?

@@ -334,6 +334,14 @@ packages:
url: "https://pub.dev"
source: hosted
version: "0.17.3"
cupertino_icons:
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, all the pubspec.lock changes should be reverted unless there was an explicit update to the packages!

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