Skip to content

feat(SponsorBlock): Add undo automatic skip button #5145

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ public class SegmentPlaybackController {
@Nullable
private static String timeWithoutSegments;

@Nullable
private static SponsorSegment lastAutoSkippedSegment;
private static long lastAutoSkippedSegmentStartTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need for a long field, when lastAutoSkippedSegment already has the start time.


private static int sponsorBarAbsoluteLeft;
private static int sponsorAbsoluteBarRight;
private static int sponsorBarThickness;
Expand Down Expand Up @@ -171,6 +175,8 @@ private static void clearData() {
toastSegmentSkipped = null;
toastNumberOfSegmentsSkipped = 0;
hiddenSkipSegmentsForCurrentVideoTime.clear();
lastAutoSkippedSegment = null;
lastAutoSkippedSegmentStartTime = 0;
}

/**
Expand Down Expand Up @@ -545,6 +551,9 @@ private static void skipSegment(@NonNull SponsorSegment segmentToSkip, boolean u

final boolean videoIsPaused = VideoState.getCurrent() == VideoState.PAUSED;
if (!userManuallySkipped) {
lastAutoSkippedSegment = segmentToSkip;
lastAutoSkippedSegmentStartTime = segmentToSkip.start;
SponsorBlockViewController.showUndoSkipButton(); // Show undo button after auto skip
Copy link
Contributor

Choose a reason for hiding this comment

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

This functionality should be a setting, and should be default off since the only default auto skip is sponsors (which most users never want to watch, and thus an undo button is unnecessary on screen clutter).

// check for any smaller embedded segments, and count those as autoskipped
final boolean showSkipToast = Settings.SB_TOAST_ON_SKIP.get();
for (final SponsorSegment otherSegment : Objects.requireNonNull(segments)) {
Expand All @@ -562,6 +571,11 @@ private static void skipSegment(@NonNull SponsorSegment segmentToSkip, boolean u
}
}
}
} else {
// If it was a manual skip, clear the auto-skipped info and hide undo button
lastAutoSkippedSegment = null;
lastAutoSkippedSegmentStartTime = 0;
SponsorBlockViewController.hideUndoSkipButton();
}

if (segmentToSkip.category == SegmentCategory.UNSUBMITTED) {
Expand Down Expand Up @@ -624,6 +638,29 @@ public static void onSkipSegmentClicked(@NonNull SponsorSegment segment) {
}
}

public static void undoLastAutoSkip() {
try {
if (lastAutoSkippedSegment == null) {
Logger.printDebug(() -> "No segment to undo skip.");
return;
}

Logger.printDebug(() -> "Undoing last auto-skipped segment: " + lastAutoSkippedSegment);
final boolean seekSuccessful = VideoInformation.seekTo(lastAutoSkippedSegmentStartTime);

if (seekSuccessful) {
// Clear the stored segment after successful undo
lastAutoSkippedSegment = null;
lastAutoSkippedSegmentStartTime = 0;
SponsorBlockViewController.hideUndoSkipButton(); // Will implement this next
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the undo button set hidden? Does it remain on screen all the time until the next segment? This should hide itself similar to the auto hiding skip button.

} else {
Logger.printDebug(() -> "Could not undo skip (seek unsuccessful): " + lastAutoSkippedSegment);
}
} catch (Exception ex) {
Logger.printException(() -> "undoLastAutoSkip failure", ex);
}
}

/**
* Injection point
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public class SkipSponsorButton extends FrameLayout {
private static final boolean highContrast = false;
private final LinearLayout skipSponsorBtnContainer;
private final TextView skipSponsorTextView;
private final ImageView skipSponsorButtonIcon; // Existing field, but adding it here for clarity in diff
Copy link
Contributor

Choose a reason for hiding this comment

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

If the field isn't used then there's no reason to have it.

private final ImageView undoButton; // New field
private final Paint background;
private final Paint border;
private SponsorSegment segment;
Expand Down Expand Up @@ -70,6 +72,8 @@ public SkipSponsorButton(Context context, AttributeSet attributeSet, int defStyl
border.setStyle(Paint.Style.STROKE);

skipSponsorTextView = Objects.requireNonNull(findViewById(getResourceIdentifier(context, "revanced_sb_skip_sponsor_button_text", "id"))); // id:skip_ad_button_text;
skipSponsorButtonIcon = Objects.requireNonNull(findViewById(getResourceIdentifier(context, "revanced_sb_skip_sponsor_button_icon", "id")));
undoButton = Objects.requireNonNull(findViewById(getResourceIdentifier(context, "revanced_sb_undo_button", "id")));
defaultBottomMargin = getResourceDimensionPixelSize("skip_button_default_bottom_margin"); // dimen:skip_button_default_bottom_margin
ctaBottomMargin = getResourceDimensionPixelSize("skip_button_cta_bottom_margin"); // dimen:skip_button_cta_bottom_margin

Expand All @@ -80,6 +84,14 @@ public SkipSponsorButton(Context context, AttributeSet attributeSet, int defStyl
setVisibility(View.GONE);
SegmentPlaybackController.onSkipSegmentClicked(segment);
});

undoButton.setOnClickListener(v -> {
SegmentPlaybackController.undoLastAutoSkip();
});
}

public ImageView getUndoButton() {
return undoButton;
}

@Override // android.view.ViewGroup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class SponsorBlockViewController {
private static WeakReference<ViewGroup> youtubeOverlaysLayoutRef = new WeakReference<>(null);
private static WeakReference<SkipSponsorButton> skipHighlightButtonRef = new WeakReference<>(null);
private static WeakReference<SkipSponsorButton> skipSponsorButtonRef = new WeakReference<>(null);
private static WeakReference<ImageView> undoButtonRef = new WeakReference<>(null); // New field
private static WeakReference<NewSegmentLayout> newSegmentLayoutRef = new WeakReference<>(null);
private static boolean canShowViewElements;
private static boolean newSegmentLayoutVisible;
Expand Down Expand Up @@ -89,6 +90,10 @@ public void onChildViewRemoved(View parent, View child) {
skipSponsorButtonRef = new WeakReference<>(Objects.requireNonNull(
layout.findViewById(getResourceIdentifier("revanced_sb_skip_sponsor_button", "id"))));

// Get reference to undo button
undoButtonRef = new WeakReference<>(Objects.requireNonNull(
skipSponsorButtonRef.get().getUndoButton()));

NewSegmentLayout newSegmentLayout = Objects.requireNonNull(
layout.findViewById(getResourceIdentifier("revanced_sb_new_segment_view", "id")));
newSegmentLayoutRef = new WeakReference<>(newSegmentLayout);
Expand All @@ -106,6 +111,7 @@ public static void hideAll() {
hideSkipHighlightButton();
hideSkipSegmentButton();
hideNewSegmentLayout();
hideUndoSkipButton();
}

public static void updateLayout() {
Expand Down Expand Up @@ -146,6 +152,14 @@ public static void hideSkipSegmentButton() {
updateSkipButton(skipSponsorButtonRef.get(), null, false);
}

public static void showUndoSkipButton() {
setViewVisibility(undoButtonRef.get(), true);
}

public static void hideUndoSkipButton() {
setViewVisibility(undoButtonRef.get(), false);
}

private static void updateSkipButton(@Nullable SkipSponsorButton button,
@Nullable SponsorSegment segment, boolean visible) {
if (button == null) {
Expand Down Expand Up @@ -202,6 +216,9 @@ private static void playerTypeChanged(@NonNull PlayerType playerType) {
SkipSponsorButton skipSponsorButton = skipSponsorButtonRef.get();
setSkipButtonMargins(skipSponsorButton, isWatchFullScreen);
setViewVisibility(skipSponsorButton, skipSegment != null);

// Hide undo button on player type change
setViewVisibility(undoButtonRef.get(), false);
} catch (Exception ex) {
Logger.printException(() -> "Player type changed failure", ex);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="utf-8"?>
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:fillColor="#FFFFFF"
android:pathData="M12.5 8c-2.65 0-5.05.99-6.9 2.6L2 7v6h6l-2.91-2.91c1.17-1.17 2.71-1.9 4.41-1.9 3.31 0 6 2.69 6 6s-2.69 6-6 6c-1.66 0-3.1-.69-4.18-1.82L7.2 16.2c.75.75 1.79 1.22 2.8 1.34V19c-2.02-.22-3.8-.95-5.18-2.08L2 19v-6h6l-2.91 2.91c1.17 1.17 2.71 1.9 4.41 1.9 3.31 0 6-2.69 6-6s-2.69-6-6-6z" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a screenshot of what this looks like in app.

</vector>
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,17 @@
android:paddingTop="3dp"
android:paddingBottom="3dp"
android:src="@drawable/quantum_ic_skip_next_white_24" />

<ImageView
android:id="@+id/revanced_sb_undo_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_vertical"
android:alpha="0.8"
android:contentDescription="@null"
android:paddingTop="3dp"
android:paddingBottom="3dp"
android:src="@drawable/revanced_sb_undo"
android:visibility="gone" />
</LinearLayout>
</merge>
</merge>