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

Nazal #10

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
walk.py
3 changes: 3 additions & 0 deletions pinne-cheyyam/background.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
chrome.runtime.onInstalled.addListener(() => {
console.log('Useless Projects Extension installed.');
});
70 changes: 70 additions & 0 deletions pinne-cheyyam/content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
function createOverlay() {
// Check if the overlay already exists
if (document.getElementById("productive-overlay")) return;

// Create the overlay div
const overlay = document.createElement("div");
overlay.id = "productive-overlay";
overlay.style.position = "fixed";
overlay.style.top = "0";
overlay.style.left = "0";
overlay.style.width = "100%";
overlay.style.height = "100%";
overlay.style.backgroundColor = "rgba(0, 0, 0, 0.8)";
overlay.style.zIndex = "10000";
overlay.style.display = "flex";
overlay.style.justifyContent = "center";
overlay.style.alignItems = "center";
overlay.style.color = "white";
overlay.style.fontSize = "24px";
overlay.style.textAlign = "center";

// Message content
overlay.innerHTML = `
<div>
<p>You are on a productive site! Take a break?</p>
<button id="close-overlay" style="
margin-top: 20px;
padding: 10px 20px;
font-size: 18px;
background-color: #176B87;
color: white;
border: none;
border-radius: 5px;
cursor: pointer;
">Close</button>
</div>
`;

// Append overlay to body
document.body.appendChild(overlay);

// Close overlay button functionality
document.getElementById("close-overlay").addEventListener("click", () => {
overlay.remove();
});
}
Comment on lines +1 to +46
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor overlay implementation for better maintainability and extensibility.

The overlay implementation has several areas for improvement:

  1. Move styles to a separate CSS file
  2. Externalize text for internationalization
  3. Clean up event listeners properly

Consider refactoring the code as follows:

+const OVERLAY_TEXT = chrome.i18n.getMessage('overlayText') || 'You are on a productive site! Take a break?';
+const CLOSE_BUTTON_TEXT = chrome.i18n.getMessage('closeButton') || 'Close';

 function createOverlay() {
     if (document.getElementById("productive-overlay")) return;
 
     const overlay = document.createElement("div");
     overlay.id = "productive-overlay";
-    overlay.style.position = "fixed";
-    // ... remove all inline styles
+    overlay.classList.add('productive-overlay');
     
     overlay.innerHTML = `
       <div>
-        <p>You are on a productive site! Take a break?</p>
-        <button id="close-overlay" style="...">Close</button>
+        <p>${OVERLAY_TEXT}</p>
+        <button id="close-overlay" class="overlay-button">${CLOSE_BUTTON_TEXT}</button>
       </div>
     `;
 
     document.body.appendChild(overlay);
 
-    document.getElementById("close-overlay").addEventListener("click", () => {
+    const closeButton = document.getElementById("close-overlay");
+    const handleClose = () => {
+      closeButton.removeEventListener("click", handleClose);
       overlay.remove();
-    });
+    };
+    closeButton.addEventListener("click", handleClose);
 }

Add a new CSS file:

.productive-overlay {
  position: fixed;
  top: 0;
  left: 0;
  width: 100%;
  height: 100%;
  background-color: rgba(0, 0, 0, 0.8);
  z-index: var(--overlay-z-index, 10000);
  display: flex;
  justify-content: center;
  align-items: center;
  color: white;
  font-size: 24px;
  text-align: center;
}

.overlay-button {
  margin-top: 20px;
  padding: 10px 20px;
  font-size: 18px;
  background-color: var(--button-bg-color, #176B87);
  color: white;
  border: none;
  border-radius: 5px;
  cursor: pointer;
}


function checkSiteType() {
const currentUrl = window.location.href;

// Retrieve the site lists from storage
chrome.storage.sync.get(['productiveSites', 'distractingSites'], (data) => {
const productiveSites = data.productiveSites || [];
const distractingSites = data.distractingSites || [];

// Check if the current URL matches any productive site
if (productiveSites.some(site => currentUrl.includes(site))) {
createOverlay(); // Display the overlay for productive sites
}

// Check if the current URL matches any distracting site
if (distractingSites.some(site => currentUrl.includes(site))) {
alert("You are on a distracting site!");
}
});
}
Comment on lines +48 to +66
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve URL matching and error handling in site type checking.

The current implementation has several potential issues:

  1. Simple string inclusion for URL matching could lead to false positives
  2. Missing error handling for storage access
  3. Alert dialogs can be intrusive

Consider implementing these improvements:

 function checkSiteType() {
     const currentUrl = window.location.href;
+    const urlObj = new URL(currentUrl);
+    const domain = urlObj.hostname;
 
     // Retrieve the site lists from storage
     chrome.storage.sync.get(['productiveSites', 'distractingSites'], (data) => {
+      if (chrome.runtime.lastError) {
+        console.error('Error accessing storage:', chrome.runtime.lastError);
+        return;
+      }
+
       const productiveSites = data.productiveSites || [];
       const distractingSites = data.distractingSites || [];
 
       // Check if the current URL matches any productive site
-      if (productiveSites.some(site => currentUrl.includes(site))) {
+      if (productiveSites.some(site => domain.endsWith(site))) {
         createOverlay();
       }
 
       // Check if the current URL matches any distracting site
-      if (distractingSites.some(site => currentUrl.includes(site))) {
-        alert("You are on a distracting site!");
+      if (distractingSites.some(site => domain.endsWith(site))) {
+        // Use a less intrusive notification method
+        chrome.runtime.sendMessage({
+          type: 'showNotification',
+          message: 'You are on a distracting site!'
+        });
       }
     });
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function checkSiteType() {
const currentUrl = window.location.href;
// Retrieve the site lists from storage
chrome.storage.sync.get(['productiveSites', 'distractingSites'], (data) => {
const productiveSites = data.productiveSites || [];
const distractingSites = data.distractingSites || [];
// Check if the current URL matches any productive site
if (productiveSites.some(site => currentUrl.includes(site))) {
createOverlay(); // Display the overlay for productive sites
}
// Check if the current URL matches any distracting site
if (distractingSites.some(site => currentUrl.includes(site))) {
alert("You are on a distracting site!");
}
});
}
function checkSiteType() {
const currentUrl = window.location.href;
const urlObj = new URL(currentUrl);
const domain = urlObj.hostname;
// Retrieve the site lists from storage
chrome.storage.sync.get(['productiveSites', 'distractingSites'], (data) => {
if (chrome.runtime.lastError) {
console.error('Error accessing storage:', chrome.runtime.lastError);
return;
}
const productiveSites = data.productiveSites || [];
const distractingSites = data.distractingSites || [];
// Check if the current URL matches any productive site
if (productiveSites.some(site => domain.endsWith(site))) {
createOverlay();
}
// Check if the current URL matches any distracting site
if (distractingSites.some(site => domain.endsWith(site))) {
// Use a less intrusive notification method
chrome.runtime.sendMessage({
type: 'showNotification',
message: 'You are on a distracting site!'
});
}
});
}


// Run the function when the page loads
window.addEventListener('load', checkSiteType);
Comment on lines +68 to +69
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve event handling for URL checks.

The current implementation might miss URL changes in single-page applications (SPAs) and could potentially run multiple times.

Consider using the following approach:

-window.addEventListener('load', checkSiteType);
+// Check on initial load
+checkSiteType();
+
+// Listen for URL changes (for SPAs)
+const observer = new MutationObserver(() => {
+  if (document.location.href !== lastUrl) {
+    lastUrl = document.location.href;
+    checkSiteType();
+  }
+});
+
+let lastUrl = document.location.href;
+observer.observe(document, { subtree: true, childList: true });
+
+// Cleanup when the content script is unloaded
+window.addEventListener('unload', () => {
+  observer.disconnect();
+});

Committable suggestion skipped: line range outside the PR's diff.


Binary file added pinne-cheyyam/icons/icon.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
29 changes: 29 additions & 0 deletions pinne-cheyyam/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"manifest_version": 2,
"name": "Useless Projects Extension",
"version": "1.0",
"description": "Annoy users with quizzes, puzzles, and mini-games when they are productive.",
"icons": {
"48": "icons/icon.png"
},
"permissions": [
"tabs",
"storage",
"activeTab"
],
"background": {
"scripts": ["background.js"],
"persistent": false
},
"content_scripts": [
{
"matches": ["<all_urls>"],
"js": ["content.js"]
}
],
"browser_action": {
"default_popup": "popup/popup.html",
"default_icon": "icons/icon.png"
},
"options_page": "options/options.html"
}
80 changes: 80 additions & 0 deletions pinne-cheyyam/options/options.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/* General Reset */
* {
margin: 0;
padding: 0;
box-sizing: border-box;
}

/* Options Page Container */
body {
font-family: Arial, sans-serif;
background-color: #f0f4f8;
color: #333;
padding: 20px;
max-width: 400px;
margin: auto;
}

h1, h2 {
color: #176B87;
font-weight: normal;
margin-bottom: 10px;
}

h2 {
font-size: 1.1rem;
margin-top: 20px;
}

/* Input and Button */
input[type="text"] {
width: 80%;
padding: 8px;
margin-top: 5px;
margin-bottom: 10px;
border: 1px solid #ddd;
border-radius: 5px;
}

button {
background-color: #176B87;
color: #fff;
border: none;
border-radius: 5px;
padding: 8px 15px;
font-size: 0.9rem;
cursor: pointer;
transition: background-color 0.3s;
margin-left: 5px;
}

button:hover {
background-color: #64CCC5;
}

button:active {
background-color: #04364A;
}

/* List of Sites */
ul {
list-style: none;
padding: 0;
}

li {
background-color: #ffffff;
border: 1px solid #ddd;
padding: 8px;
margin-bottom: 5px;
border-radius: 5px;
display: flex;
justify-content: space-between;
align-items: center;
}

li button {
padding: 5px 10px;
font-size: 0.8rem;
margin: 0;
}
27 changes: 27 additions & 0 deletions pinne-cheyyam/options/options.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Options - Useless Projects Extension</title>
<link rel="stylesheet" href="options.css">
</head>
<body>
<h1>Manage Sites</h1>

<div>
<h2>Productive Sites</h2>
<input type="text" id="productiveInput" placeholder="Add a productive site URL">
<button id="addProductive">Add</button>
Comment on lines +13 to +14
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance input validation and accessibility.

The input fields and buttons need improvements for better accessibility and user experience:

  1. Add aria-label attributes to inputs for screen readers
  2. Add pattern validation for URL inputs
  3. Add disabled state handling for the Add buttons

Apply these changes:

-    <input type="text" id="productiveInput" placeholder="Add a productive site URL">
-    <button id="addProductive">Add</button>
+    <input 
+      type="url" 
+      id="productiveInput" 
+      placeholder="Add a productive site URL"
+      aria-label="Enter productive site URL"
+      pattern="https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"
+    >
+    <button id="addProductive" aria-label="Add productive site">Add</button>

-    <input type="text" id="distractingInput" placeholder="Add a distracting site URL">
-    <button id="addDistracting">Add</button>
+    <input 
+      type="url" 
+      id="distractingInput" 
+      placeholder="Add a distracting site URL"
+      aria-label="Enter distracting site URL"
+      pattern="https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"
+    >
+    <button id="addDistracting" aria-label="Add distracting site">Add</button>

Also applies to: 20-21

<ul id="productiveList"></ul>
</div>

<div>
<h2>Distracting Sites</h2>
<input type="text" id="distractingInput" placeholder="Add a distracting site URL">
<button id="addDistracting">Add</button>
<ul id="distractingList"></ul>
</div>
Comment on lines +11 to +23
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding form elements for better user interaction.

Wrap the input sections in <form> elements to enable submit-on-enter functionality and proper form handling.

Apply these changes:

-  <div>
+  <form id="productiveForm">
     <h2>Productive Sites</h2>
     <!-- ... input and button ... -->
     <ul id="productiveList"></ul>
-  </div>
+  </form>

-  <div>
+  <form id="distractingForm">
     <h2>Distracting Sites</h2>
     <!-- ... input and button ... -->
     <ul id="distractingList"></ul>
-  </div>
+  </form>

Committable suggestion skipped: line range outside the PR's diff.


<script src="options.js"></script>
</body>
</html>
57 changes: 57 additions & 0 deletions pinne-cheyyam/options/options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Script to handle saving and loading options
// Helper function to update the display of the site lists
function updateSiteList(type, listId) {
chrome.storage.sync.get([type], (result) => {
const list = result[type] || [];
const listElement = document.getElementById(listId);
listElement.innerHTML = '';
list.forEach((site, index) => {
const listItem = document.createElement('li');
listItem.textContent = site;

// Add remove button
const removeButton = document.createElement('button');
removeButton.textContent = 'Remove';
removeButton.onclick = () => removeSite(type, index);
listItem.appendChild(removeButton);

listElement.appendChild(listItem);
});
});
}
Comment on lines +3 to +21
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add input validation and error handling.

The updateSiteList function needs additional safeguards:

  1. Validate the type parameter to ensure it's either 'productiveSites' or 'distractingSites'.
  2. Add error handling for storage operations.

Here's the suggested implementation:

 function updateSiteList(type, listId) {
+    if (!['productiveSites', 'distractingSites'].includes(type)) {
+        console.error('Invalid site type:', type);
+        return;
+    }
     chrome.storage.sync.get([type], (result) => {
+        if (chrome.runtime.lastError) {
+            console.error('Storage error:', chrome.runtime.lastError);
+            return;
+        }
         const list = result[type] || [];
         const listElement = document.getElementById(listId);
+        if (!listElement) {
+            console.error('List element not found:', listId);
+            return;
+        }
         listElement.innerHTML = '';
         list.forEach((site, index) => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function updateSiteList(type, listId) {
chrome.storage.sync.get([type], (result) => {
const list = result[type] || [];
const listElement = document.getElementById(listId);
listElement.innerHTML = '';
list.forEach((site, index) => {
const listItem = document.createElement('li');
listItem.textContent = site;
// Add remove button
const removeButton = document.createElement('button');
removeButton.textContent = 'Remove';
removeButton.onclick = () => removeSite(type, index);
listItem.appendChild(removeButton);
listElement.appendChild(listItem);
});
});
}
function updateSiteList(type, listId) {
if (!['productiveSites', 'distractingSites'].includes(type)) {
console.error('Invalid site type:', type);
return;
}
chrome.storage.sync.get([type], (result) => {
if (chrome.runtime.lastError) {
console.error('Storage error:', chrome.runtime.lastError);
return;
}
const list = result[type] || [];
const listElement = document.getElementById(listId);
if (!listElement) {
console.error('List element not found:', listId);
return;
}
listElement.innerHTML = '';
list.forEach((site, index) => {
const listItem = document.createElement('li');
listItem.textContent = site;
// Add remove button
const removeButton = document.createElement('button');
removeButton.textContent = 'Remove';
removeButton.onclick = () => removeSite(type, index);
listItem.appendChild(removeButton);
listElement.appendChild(listItem);
});
});
}


// Function to add a new site
function addSite(type, inputId, listId) {
const inputElement = document.getElementById(inputId);
const site = inputElement.value.trim();
if (site) {
chrome.storage.sync.get([type], (result) => {
const sites = result[type] || [];
sites.push(site);
chrome.storage.sync.set({ [type]: sites }, () => {
updateSiteList(type, listId);
inputElement.value = '';
});
});
}
}
Comment on lines +24 to +37
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance input validation and add duplicate checking.

The addSite function needs improvements:

  1. Validate URLs before adding.
  2. Prevent duplicate entries.
  3. Add a reasonable limit to the number of sites.
  4. Include error handling.

Here's the suggested implementation:

 function addSite(type, inputId, listId) {
     const inputElement = document.getElementById(inputId);
     const site = inputElement.value.trim();
-    if (site) {
+    try {
+        // Validate URL format
+        new URL(site);
+    } catch (e) {
+        alert('Please enter a valid URL');
+        return;
+    }
+
+    if (site) {
         chrome.storage.sync.get([type], (result) => {
+            if (chrome.runtime.lastError) {
+                console.error('Storage error:', chrome.runtime.lastError);
+                return;
+            }
             const sites = result[type] || [];
+            
+            // Check for duplicates
+            if (sites.includes(site)) {
+                alert('This site is already in the list');
+                return;
+            }
+            
+            // Limit number of sites
+            if (sites.length >= 50) {
+                alert('Maximum number of sites reached');
+                return;
+            }
             sites.push(site);
             chrome.storage.sync.set({ [type]: sites }, () => {
+                if (chrome.runtime.lastError) {
+                    console.error('Storage error:', chrome.runtime.lastError);
+                    return;
+                }
                 updateSiteList(type, listId);
                 inputElement.value = '';
             });
         });
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function addSite(type, inputId, listId) {
const inputElement = document.getElementById(inputId);
const site = inputElement.value.trim();
if (site) {
chrome.storage.sync.get([type], (result) => {
const sites = result[type] || [];
sites.push(site);
chrome.storage.sync.set({ [type]: sites }, () => {
updateSiteList(type, listId);
inputElement.value = '';
});
});
}
}
function addSite(type, inputId, listId) {
const inputElement = document.getElementById(inputId);
const site = inputElement.value.trim();
try {
// Validate URL format
new URL(site);
} catch (e) {
alert('Please enter a valid URL');
return;
}
if (site) {
chrome.storage.sync.get([type], (result) => {
if (chrome.runtime.lastError) {
console.error('Storage error:', chrome.runtime.lastError);
return;
}
const sites = result[type] || [];
// Check for duplicates
if (sites.includes(site)) {
alert('This site is already in the list');
return;
}
// Limit number of sites
if (sites.length >= 50) {
alert('Maximum number of sites reached');
return;
}
sites.push(site);
chrome.storage.sync.set({ [type]: sites }, () => {
if (chrome.runtime.lastError) {
console.error('Storage error:', chrome.runtime.lastError);
return;
}
updateSiteList(type, listId);
inputElement.value = '';
});
});
}
}


// Function to remove a site
function removeSite(type, index) {
chrome.storage.sync.get([type], (result) => {
const sites = result[type] || [];
sites.splice(index, 1);
chrome.storage.sync.set({ [type]: sites }, () => {
updateSiteList(type, type === 'productiveSites' ? 'productiveList' : 'distractingList');
});
});
}
Comment on lines +40 to +48
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling and use constants for list IDs.

The removeSite function needs improvements:

  1. Add error handling.
  2. Use constants for list IDs.
  3. Validate index before removal.

Here's the suggested implementation:

+const LIST_IDS = {
+    productiveSites: 'productiveList',
+    distractingSites: 'distractingList'
+};
+
 function removeSite(type, index) {
+    if (!['productiveSites', 'distractingSites'].includes(type)) {
+        console.error('Invalid site type:', type);
+        return;
+    }
     chrome.storage.sync.get([type], (result) => {
+        if (chrome.runtime.lastError) {
+            console.error('Storage error:', chrome.runtime.lastError);
+            return;
+        }
         const sites = result[type] || [];
+        if (index < 0 || index >= sites.length) {
+            console.error('Invalid index:', index);
+            return;
+        }
         sites.splice(index, 1);
         chrome.storage.sync.set({ [type]: sites }, () => {
-            updateSiteList(type, type === 'productiveSites' ? 'productiveList' : 'distractingList');
+            if (chrome.runtime.lastError) {
+                console.error('Storage error:', chrome.runtime.lastError);
+                return;
+            }
+            updateSiteList(type, LIST_IDS[type]);
         });
     });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function removeSite(type, index) {
chrome.storage.sync.get([type], (result) => {
const sites = result[type] || [];
sites.splice(index, 1);
chrome.storage.sync.set({ [type]: sites }, () => {
updateSiteList(type, type === 'productiveSites' ? 'productiveList' : 'distractingList');
});
});
}
const LIST_IDS = {
productiveSites: 'productiveList',
distractingSites: 'distractingList'
};
function removeSite(type, index) {
if (!['productiveSites', 'distractingSites'].includes(type)) {
console.error('Invalid site type:', type);
return;
}
chrome.storage.sync.get([type], (result) => {
if (chrome.runtime.lastError) {
console.error('Storage error:', chrome.runtime.lastError);
return;
}
const sites = result[type] || [];
if (index < 0 || index >= sites.length) {
console.error('Invalid index:', index);
return;
}
sites.splice(index, 1);
chrome.storage.sync.set({ [type]: sites }, () => {
if (chrome.runtime.lastError) {
console.error('Storage error:', chrome.runtime.lastError);
return;
}
updateSiteList(type, LIST_IDS[type]);
});
});
}


// Event listeners for adding sites
document.getElementById('addProductive').addEventListener('click', () => addSite('productiveSites', 'productiveInput', 'productiveList'));
document.getElementById('addDistracting').addEventListener('click', () => addSite('distractingSites', 'distractingInput', 'distractingList'));

// Initialize lists on load
updateSiteList('productiveSites', 'productiveList');
updateSiteList('distractingSites', 'distractingList');
Comment on lines +51 to +56
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for initialization and refactor event listeners.

The initialization code could be improved with error handling and using the constants defined earlier.

Here's the suggested implementation:

-document.getElementById('addProductive').addEventListener('click', () => addSite('productiveSites', 'productiveInput', 'productiveList'));
-document.getElementById('addDistracting').addEventListener('click', () => addSite('distractingSites', 'distractingInput', 'distractingList'));
+// Initialize event listeners
+Object.entries({
+    addProductive: ['productiveSites', 'productiveInput'],
+    addDistracting: ['distractingSites', 'distractingInput']
+}).forEach(([buttonId, [type, inputId]]) => {
+    const button = document.getElementById(buttonId);
+    if (button) {
+        button.addEventListener('click', () => addSite(type, inputId, LIST_IDS[type]));
+    } else {
+        console.error(`Button not found: ${buttonId}`);
+    }
+});

-// Initialize lists on load
-updateSiteList('productiveSites', 'productiveList');
-updateSiteList('distractingSites', 'distractingList');
+// Initialize lists on load
+document.addEventListener('DOMContentLoaded', () => {
+    Object.entries(LIST_IDS).forEach(([type, listId]) => {
+        updateSiteList(type, listId);
+    });
+});

Committable suggestion skipped: line range outside the PR's diff.


32 changes: 32 additions & 0 deletions pinne-cheyyam/popup/popup.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<title>Pinne-Cheyyam</title>
<!-- Link to your local Tailwind CSS file -->
<script src="../tailwind.js"></script>
Comment on lines +7 to +8
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider using a CDN or proper build process for Tailwind CSS.

Loading Tailwind directly via a script tag is not recommended for production. Consider using either:

  • A CDN link for development
  • A proper build process to generate optimized CSS
-  <!-- Link to your local Tailwind CSS file -->
-  <script src="../tailwind.js"></script>
+  <link href="https://cdn.jsdelivr.net/npm/[email protected]/dist/tailwind.min.css" rel="stylesheet">

Committable suggestion skipped: line range outside the PR's diff.

</head>

<body class="bg-gradient-to-br from-purple-700 to-blue-500 text-white min-h-screen flex items-center justify-center">
<div class="w-80 bg-white bg-opacity-10 backdrop-filter backdrop-blur-lg rounded-lg shadow-lg p-6 flex flex-col items-center">
<header class="flex flex-col items-center mb-6">
<img src="../icons/icon.png" alt="Pinne-Cheyyam Logo" class="w-16 h-16 mb-2">
<h1 class="text-2xl font-bold text-yellow-400">Pinne-Cheyyam</h1>
</header>
<main class="text-center mb-6">
<p class="mb-6 text-lg">Stay productive or get distracted!</p>
<!-- Start Annoying Button -->
<button id="start" class="bg-yellow-500 hover:bg-yellow-600 text-white font-bold py-2 px-4 rounded-full mb-4 transition duration-300 ease-in-out shadow-md">
Start Annoying
</button>
<!-- Settings Button -->
<button id="openOptions" class="bg-transparent border border-yellow-500 hover:bg-yellow-500 hover:text-white text-yellow-500 font-bold py-2 px-4 rounded-full transition duration-300 ease-in-out shadow-md">
Settings
</button>
</main>
</div>
<script src="popup.js"></script>
</body>

</html>
8 changes: 8 additions & 0 deletions pinne-cheyyam/popup/popup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
document.getElementById('start').addEventListener('click', () => {
console.log('Annoying started!');
});
Comment on lines +1 to +3
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling and remove console.log

The current implementation has two issues:

  1. Missing error handling for the DOM element lookup
  2. Console.log statement should not be in production code

Consider implementing proper error handling and actual functionality:

-document.getElementById('start').addEventListener('click', () => {
-  console.log('Annoying started!');
-});
+const startButton = document.getElementById('start');
+if (startButton) {
+  startButton.addEventListener('click', () => {
+    // TODO: Implement the actual start functionality
+  });
+} else {
+  console.error('Start button not found in popup');
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
document.getElementById('start').addEventListener('click', () => {
console.log('Annoying started!');
});
const startButton = document.getElementById('start');
if (startButton) {
startButton.addEventListener('click', () => {
// TODO: Implement the actual start functionality
});
} else {
console.error('Start button not found in popup');
}


document.getElementById('openOptions').addEventListener('click', () => {
// Opens the options page
chrome.runtime.openOptionsPage();
});
Comment on lines +5 to +8
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for options button

Similar to the start button, this needs proper error handling for the DOM element lookup.

Apply this improvement:

-document.getElementById('openOptions').addEventListener('click', () => {
-  // Opens the options page
-  chrome.runtime.openOptionsPage();
-});
+const optionsButton = document.getElementById('openOptions');
+if (optionsButton) {
+  optionsButton.addEventListener('click', () => {
+    chrome.runtime.openOptionsPage();
+  });
+} else {
+  console.error('Options button not found in popup');
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
document.getElementById('openOptions').addEventListener('click', () => {
// Opens the options page
chrome.runtime.openOptionsPage();
});
const optionsButton = document.getElementById('openOptions');
if (optionsButton) {
optionsButton.addEventListener('click', () => {
chrome.runtime.openOptionsPage();
});
} else {
console.error('Options button not found in popup');
}

Comment on lines +1 to +8
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing implementation for 'start' functionality

The background script currently only handles installation events and lacks any implementation for the 'start' functionality referenced in popup.js. The click handler in popup.js only logs a message without any actual integration with the background script.

Required changes:

  • Implement message passing between popup and background script for the 'start' functionality
  • Add corresponding message handler in background.js to process the 'start' action
🔗 Analysis chain

Verify integration with background script

The 'start' functionality seems to be missing its core implementation. We should verify if there's corresponding handling in the background script.

Let's check for related background script implementations:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Look for background script handling of the 'start' functionality

# Check for background script files
echo "Checking for background script files:"
fd -e js -e ts background

# Look for message handling related to 'start' functionality
echo -e "\nChecking for message handlers:"
rg -l "chrome\.(runtime|extension)\.onMessage" 

Length of output: 266


Script:

#!/bin/bash
# Let's examine the content of the background script and look for any start-related functionality
echo "Content of background.js:"
cat pinne-cheyyam/background.js

# Also check for any other potential message passing or start functionality implementations
echo -e "\nChecking for any start-related functionality across the codebase:"
rg -i "start" --type js -C 2

Length of output: 600

81 changes: 81 additions & 0 deletions pinne-cheyyam/tailwind.js

Large diffs are not rendered by default.