SponsorBlock icon indicating copy to clipboard operation
SponsorBlock copied to clipboard

Implement Channel-Specific Category Selection

Open zedseven opened this issue 3 years ago • 10 comments

Adds channel-specific category selection, and makes the improvements necessary under the hood to make it possible. This PR was created with a lot of @mchangrh's help. Thank you.

This adds ctrl-click functionality to the whitelist button in the popup. This opens channel-specific settings and creates a new channel entry if it's not present.

In order for this to work properly, it requires the Force Channel Check Before Skipping option to be enabled.

This was my first time working with TypeScript and React, so please let me know if I've done something incorrectly.

This PR is the reason for #1249 and #1250.

Notes

This PR makes the following changes to support channel-specific categories:

  1. Removes the whitelistedChannels config property, integrates channel whitelisting with the new channelSpecificSettings property, and migrates old configs to the new format.
  2. Adds a new Disabled enum option for CategorySkipOption, which is set to -1. (so it doesn't mess up existing option values stored in configs) This was required so that categories could be disabled for specific channels, since previously the extension disabled categories by removing them from the list. (which doesn't work for inherited values)
  3. Fetches the channel name at the same time as when it fetches the channel ID. This is used for display in the channel dropdown.
  4. The Behavior table header now has a defined height, so it doesn't change height when the two-line colour headings disappear.
  5. The hover text for the whitelist button in the popup has been changed, since there's already a notice when a channel is first whitelisted.
  6. Fixes a bug I found where shouldSkip would return true for full-video segments on music videos if autoSkipOnMusicVideos was enabled.

New Localisation Values

The following values are new and at the moment only have en localisation keys:

  • removeChannelSettingsButton: The label of the button for removing channel-specific settings.
  • inherit: The Inherit skip option for channel-specific category selections.
  • channelSettingsPopup: The hover text for the whitelist button that mentions to Ctrl-click the button for channel-specific settings.
  • forceChannelCheckRequired: The warning that channel-specific settings don't work properly if Force Channel Check Before Skipping is not enabled.

Known Jank

  1. More could probably be done to tell the user about the ctrl-click functionality, but I felt this was a good start.
  2. In the Behavior tab, scroll up the page a bit. Select a channel in the dropdown - the page will not scroll or move. If you now select the Global settings again, the page does scroll. It's the inconsistency that bothers me, not the scrolling. I have a feeling it's something to do with React DOM updates related to the re-appearing colour selection elements, but I'm not sure how or why.

  • [x] I agree to license my contribution under LGPL-3.0 or my contribution is from another project with a license compatible with LGPL-3.0

To test this pull request, follow the instructions in the wiki.

zedseven avatar Apr 03 '22 01:04 zedseven

Awesome job here, thank you so much for implementing this! Hope it gets merged soon! ❤️

Oliver-Saer avatar Apr 15 '22 14:04 Oliver-Saer

In order for this to work properly, it requires the Force Channel Check Before Skipping option to be enabled.

Solution: Whenever a setting is changed, update a new list that saves the list of the union of all enabled categories. So, if filler is only enabled for one channel, still include it in that list. Use that list for all fetches

ajayyy avatar Apr 19 '22 17:04 ajayyy

Very nice implementation!

  • Whitelisting probably shouldn't auto-create a new channel config, only ctrl-click. Right now, whitelisting and then unwhitelisting leaves a zombie config in the options.

  • Maybe whitelisting should be removed entirely and replaced with this. Old whitelists still should be converted, but we can rename it to "Channel Overrides", and a single click opens the options page.

  • Either the options below the category selection need to also be synced with channels, or we should hide them when switching off global config to prevent confusion

ajayyy avatar Apr 22 '22 19:04 ajayyy

Thank you!

I just wanted to mention that I haven't forgotten about this and I'm hoping to look at it next week. Just been very busy with work.

zedseven avatar May 06 '22 16:05 zedseven

I've just tested it and it's working great for me. Thank you!

wolph avatar May 06 '22 22:05 wolph

To test this pull request, follow the instructions in the wiki.

Artifacts expired i cannot download :(

Splarkszter avatar Aug 06 '22 14:08 Splarkszter

@zedseven

Thank you!

I just wanted to mention that I haven't forgotten about this and I'm hoping to look at it next week. Just been very busy with work.

Little bump. This feature would be very useful :)

wolph avatar Apr 09 '23 20:04 wolph

Little bump. This feature would be very useful :)

+1

Splarkszter avatar Apr 09 '23 22:04 Splarkszter

To test this pull request, follow the instructions in the wiki.

Artifacts expired i cannot download :(

Yeah

PasttaTypo avatar Dec 20 '23 16:12 PasttaTypo

@zedseven any news?

eirnym avatar Jan 25 '24 20:01 eirnym