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

Omnibox keyword search fix, DIY sidePanel, and PowerShell build script #102

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft
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
19 changes: 19 additions & 0 deletions build.ps1
Copy link
Author

Choose a reason for hiding this comment

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

sorry, I can't run sh script on windows that's why i added

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Install deps
npm install

# Build
npm run build

# Check if --firefox argument was passed
if ($args[0] -eq "--firefox") {
# Copy to firefox/manifest.json
Write-Host "Built for Firefox..."
Copy-Item -Path "firefox/manifest.json" -Destination "dist/manifest.json"
} else {
# Copy to dist/manifest.json
Write-Host "Built for Chrom(ium)e..."
Copy-Item -Path "src/manifest.json" -Destination "dist/manifest.json"
}

# Done (for now...)
Write-Host "Done! "
9 changes: 7 additions & 2 deletions src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
"page": "./src/pages/Options/options.html",
"browser_style": false
},
"side_panel": {
"default_path": "./index.html"
},
"icons": {
"16": "./16.png",
"32": "./32.png",
Expand All @@ -30,14 +33,16 @@
"tabs",
"bookmarks",
"commands",
"contextMenus"
"contextMenus",
"sidePanel"

],
"background": {
"service_worker": "./background.js",
"type": "module"
},
"omnibox": {
"keyword": "lk"
"keyword": "lw"
Copy link
Author

Choose a reason for hiding this comment

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

should be "lw" considering it is linkwarden and not linkding's "lk"

},
"host_permissions": [
"*://*/*"
Expand Down
28 changes: 27 additions & 1 deletion src/pages/Background/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import OnClickData = chrome.contextMenus.OnClickData;
import OnInputEnteredDisposition = chrome.omnibox.OnInputEnteredDisposition;

const browser = getBrowser();
let configuration = null; // this js needs init function which initializes the configuration variable thus solving many lines in this file which repeadiatly uses getConfig() it might be better
Copy link
Author

Choose a reason for hiding this comment

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

i come from the java/kotlin/python so I don't know more of vite/javascript/typescript but this variable needs to be initialized in init function which should be added to this file which also reduces the getConfig() function code.


// This is the main functions that will be called when a bookmark is created, update or deleted
// Won't work with axios xhr or something not supported by the browser
Expand Down Expand Up @@ -236,10 +237,12 @@ browser.omnibox.onInputEntered.addListener(async (content: string, disposition:
return;
}

configuration = await getConfig(); // code line waiting to be replaced by nothing once global init function is done until then mae do with this

const isUrl = /^http(s)?:\/\//.test(content);
const url = isUrl
? content
: `lk`;
: `${configuration.baseUrl}/search?q=${encodeURIComponent(content)}`; // This part was taken https://github.com/sissbruecker/linkding-extension/blob/master/src/background.js
Copy link
Author

Choose a reason for hiding this comment

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

code from linkding


// Edge doesn't allow updating the New Tab Page (tested with version 117).
// Trying to do so will throw: "Error: Cannot update NTP tab."
Expand Down Expand Up @@ -267,3 +270,26 @@ browser.omnibox.onInputEntered.addListener(async (content: string, disposition:
});


// refresh for sidepanel on tab change
chrome.tabs.onUpdated.addListener((tabId, changeInfo) => {
// if (changeInfo.url) {
console.log("Tab URL changed:", changeInfo.url, " Tab ID:", tabId);

// Reload the side panel if needed
chrome.sidePanel.setOptions({ path: "./index.html" });
// }
});

// Listen for tab changes
chrome.tabs.onActivated.addListener(async (activeInfo) => {
console.log("Tab changed:", activeInfo);

try {
await chrome.sidePanel.setOptions({
path: "./index.html"
});
console.log("Side panel reloaded.");
} catch (error) {
console.error("Failed to reload the side panel:", error);
}
});
Copy link
Author

Choose a reason for hiding this comment

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

ummmm.... i inspected through the background.js console it does execute (see attached image) but chrome.sidePanel.setOptions() function doesn't reload the tab if it's already set to that for that we need to separate the sidepanel page and still i don't know vite so but i'll try to learn vite and implement it via chrome.runtime.onMessage.addListener() which should be able to execute the reload code window.location.reload();
📝:- also you might want to block comment out the both listener functions since they do nothing it was for debug.

image