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

Only hide thumbnails during set timeframe #22

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

Only hide thumbnails during set timeframe #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ const defaultOptions = {
everywhere: false,
},
thumbnailMode: 'hidden',
workMode: {
Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Owner

Choose a reason for hiding this comment

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

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

enabled: false,
startTime: "09:00",
endTime: "17:00",
},
}

/**
Expand Down
32 changes: 30 additions & 2 deletions inject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)=>{
Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Owner

Choose a reason for hiding this comment

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

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

currentDate = new Date();

startDate = new Date(currentDate.getTime());
startDate.setHours(startTime.split(":")[0]);
startDate.setMinutes(startTime.split(":")[1]);
startDate.setSeconds(0);

endDate = new Date(currentDate.getTime());
endDate.setHours(endTime.split(":")[0]);
endDate.setMinutes(endTime.split(":")[1]);
endDate.setSeconds(0);

return currentDate > startDate && currentDate < endDate;
}

const updateElem = async () => {
const options = await loadOptions()

Expand All @@ -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){
Copy link
Owner

Choose a reason for hiding this comment

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

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.


if(IsItWorkTime(options.workMode.startTime, options.workMode.endTime)){
elem.innerHTML = `/* Injected by the Hide YouTube Thumbnails extension */
${css[options.thumbnailMode]}`
}else{
elem.innerHTML = `/* Injected by the Hide YouTube Thumbnails extension */
${css["normal"]}`
}
}else{
elem.innerHTML = `/* Injected by the Hide YouTube Thumbnails extension */
${css[isDisabled ? 'normal' : options.thumbnailMode]}`
}

}

// Update when settings are changed
Expand Down
10 changes: 10 additions & 0 deletions options.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Owner

Choose a reason for hiding this comment

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

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

<p data-i18n="options_work_mode_description">Only hide thumbnails during a set time of the day</p>
<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" >
Copy link
Owner

Choose a reason for hiding this comment

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

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


<label for="workModeEnd">End date:</label>
<input type="time" id="workModeEnd" name="workModeEnd" >

<p id="status">✅ <span data-i18n="options_loaded">Preferences loaded</span></p>
</form>
<script src="common.js"></script>
Expand Down
15 changes: 15 additions & 0 deletions options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,23 @@ document.addEventListener('DOMContentLoaded', async () => {

// Load existing settings
const options = await loadOptions();
//Settings storage
document.forms[0].syncSettings.checked = options.syncSettings;

//Thumbnail mode
document.forms[0].thumbnailMode.value = options.thumbnailMode;

//Disabled pages
document.forms[0].disableSearchResultPage.checked = options.disabledOnPages.results;
document.forms[0].disablePlaylistPage.checked = options.disabledOnPages.playlist;
document.forms[0].disableWatchPage.checked = options.disabledOnPages.watch;
document.forms[0].disableSubscriptionsPage.checked = options.disabledOnPages.subscriptions;
document.forms[0].disableEverywhere.checked = options.disabledOnPages.everywhere;

//Work Mode
document.forms[0].enableWorkMode.checked = options.workMode.enabled;
document.forms[0].workModeStart.value = options.workMode.startTime;
document.forms[0].workModeEnd.value = options.workMode.endTime;
});

// Save on change
Expand All @@ -35,6 +45,11 @@ document.forms[0].addEventListener('change', async () => {
subscriptions: document.forms[0].disableSubscriptionsPage.checked,
everywhere: document.forms[0].disableEverywhere.checked,
},
workMode: {
enabled: document.forms[0].enableWorkMode.checked,
startTime: document.forms[0].workModeStart.value,
endTime: document.forms[0].workModeEnd.value,
},
})

// Artificial delay, so the 'saving' message actually appears
Expand Down