Only hide thumbnails during set timeframe #22

IsaiahPapa · 2023-06-10T20:43:34Z

If work mode is enabled:

If it's current work time, hide/blur/etc thumbnails.
If it's not work time, show thumbnails/disable extension

if work mode is disabled:

Extension acts normally

Added "work mode" ability to only hide thumbnails during a set timeframe. Disabled by default, default time frame from 9am-5pm.

Please check the updateElem function, because I overrode the main functionality and would be open to a better way.

Added "work mode" ability to only hide thumbnails during a set timeframe.

domdomegg · 2023-06-11T19:30:03Z

Question: Should this be part of the extension?

tldr: Yes. It's in keeping with a key use case, and the downsides are not too great.

For:

A primary use case of the extension is to improve productivity by reducing distractions. This feature ties in well with this use case, as users might want to set certain hours of the day for work / study / etc. and others where they're free to do what they want. Software that shares characteristics often have time-based blocking, e.g. LeechBlock).
We could deprecate and remove this functionality later if it does become confusing to users or is a pain to maintain.
The code changes don't look too significant, and it's therefore not that much to maintain.

Against:

More functionality can confuse users as to the purpose of the extension. Additionally, it can confuse users and result in users misconfiguring the extension. We can mitigate this somewhat by ensuring the options menu is still clear.
Adding features results in increased code complexity and maintenance burden. We can mitigate this by ensuring the code is simple and in keeping with the existing logic.

Alternatives:

Directing users to manually enable and disable the extension. This is unsuitable as the point of the extension is to make it easy for users to control distractions automatically. Having to manually enable this is will make people less likely to use the extension and get value from it.
Encouraging users to use another tool to enable/disable the extension at particular times. After a very brief search, I was unable to find a solution to easily enable/disable extensions automatically in a scheduled way.

domdomegg

Looks pretty good! A few changes requested, but concept is sound and most code good.

If you're busy, am happy to make the edits myself in the next couple of days.

Review comments key

domdomegg · 2023-06-11T19:31:30Z

common.js

@@ -28,6 +28,11 @@ const defaultOptions = {
    everywhere: false,
  },
  thumbnailMode: 'hidden',
+  workMode: {


should: call this scheduledBlocking or timeBasedBlocking, or similar. On first glance I don't think I'd know what workMode means.

must: Update the typedef on lines 9-18 to match (GitHub won't let me comment there). This ensures intellisense works as expected, and in future if we convert to TypeScript / run linters they'll pass.

You probably will want to merge in the master branch first, because I think there may be a few merge conflicts otherwise.

consider: store this as an array of enabled times. This would allow users to set multiple periods where the extension is enabled, for example if they have a break in their work day or similar.

We don't actually have to accept more than one to start with, but I think it'd be good to design this so that if we do want to accept more than one in future people don't lose their settings. The options page would either:

have an input as just a text field, where people can enter comma-separated ranges - format could be like LeechBlock (e.g. 0900-1700,1800-1900) or ISO8601-like format (e.g. 09:00/17:00,18:00/19:00)

have pairs of time inputs, and users could click a + icon to add more ranges

domdomegg · 2023-06-11T19:46:10Z

inject.js

@@ -56,8 +72,20 @@ const updateElem = async () => {
    || (options.disabledOnPages.watch && window.location.pathname === '/watch')
    || (options.disabledOnPages.subscriptions && window.location.pathname === '/feed/subscriptions');

-  elem.innerHTML = `/* Injected by the Hide YouTube Thumbnails extension */
-  ${css[isDisabled ? 'normal' : options.thumbnailMode]}`
+  if(options.workMode.enabled){


must: I think we can move this logic into the isDisabled check. For example, adding a condition like:

|| (options.workMode.enabled && !IsItWorkTime(options.workMode.startTime, options.workMode.endTime))

This significantly reduces code duplication and complexity.

domdomegg · 2023-06-11T19:48:21Z

inject.js

@@ -47,6 +47,22 @@ ytd-playlist-video-renderer:not(:hover) ytd-thumbnail,
 const elem = document.createElement("style");
 document.documentElement.appendChild(elem);

+const IsItWorkTime = (startTime, endTime)=>{


nit: use camel case for JS functions. Align naming with options if we're renaming to scheduledBlocking.

domdomegg · 2023-06-11T19:48:40Z

inject.js

@@ -47,6 +47,22 @@ ytd-playlist-video-renderer:not(:hover) ytd-thumbnail,
 const elem = document.createElement("style");
 document.documentElement.appendChild(elem);

+const IsItWorkTime = (startTime, endTime)=>{


praise: This logic looks sound and simple. I like it!

domdomegg · 2023-06-11T19:49:07Z

options.html

+      <input type="checkbox" name="enableWorkMode" id="enableWorkMode">
+      <label for="enableWorkMode" data-i18n="options_enable_work_mode">Enabled</label><br />
+      <label for="workModeStart">Start date:</label>
+      <input type="time" id="workModeStart" name="workModeStart" >


praise: I like the use of the type="time" input!

domdomegg · 2023-06-11T19:49:52Z

options.html

@@ -36,6 +36,16 @@
      <label for="disableEverywhere" data-i18n="options_disable_extension_everywhere">Everywhere</label><br />
      <br />

+      <p data-i18n="options_work_mode">Work Mode</p>


(update naming here if we're changing it to scheduledBlocking or similar)

domdomegg · 2023-06-11T19:52:36Z

options.html

@@ -36,6 +36,16 @@
      <label for="disableEverywhere" data-i18n="options_disable_extension_everywhere">Everywhere</label><br />
      <br />

+      <p data-i18n="options_work_mode">Work Mode</p>


consider: Adding the strings to the en and es translation files in _locales. This makes things easier to translate in future.

Only hide during timeframe

324dc32

Added "work mode" ability to only hide thumbnails during a set timeframe.

domdomegg self-requested a review June 11, 2023 19:30

domdomegg requested changes Jun 11, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only hide thumbnails during set timeframe #22

Only hide thumbnails during set timeframe #22

IsaiahPapa commented Jun 10, 2023 •

edited

Loading

domdomegg commented Jun 11, 2023

domdomegg left a comment

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

domdomegg Jun 11, 2023

Only hide thumbnails during set timeframe #22

Are you sure you want to change the base?

Only hide thumbnails during set timeframe #22

Conversation

IsaiahPapa commented Jun 10, 2023 • edited Loading

domdomegg commented Jun 11, 2023

domdomegg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IsaiahPapa commented Jun 10, 2023 •

edited

Loading