-
Notifications
You must be signed in to change notification settings - Fork 9
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
Added support for the kagi search engine. #21
Conversation
Hey @danbush, thanks a lot for taken the time to get to know the codebase and make the necessary changes! I really appreciate it. I checked your PR out a bit and it looks good so far but would need a few changes. The major problem though is that kagi is behind a paywall and while there's a trial tier it's only 100 searches for one time. This is enough to test this PR but means I can't properly test or support this in the future if something breaks. The searches will eventually run out. Are you willing to become a proper maintainer (for the kagi side of things)? |
Hey! I've never been a maintainer for something beyond my personal projects (or at work), so I'm not 100% sure what I'd be signing up for, but if it would just mean updating it as needed, sure I'm willing! I made these changes mostly just for myself, so I'd be fixing it as bugs come up either way haha |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed your changes. Thanks again for the work so far.
Yes, it's just updating when something breaks for Kagi and/or testing when I change something in the injection code that might affect Kagi. |
Thanks for the additional commits. There were still a few small issues, I just took them on myself. I also made two comments in the review conversations above. Automatic theme detection should work now, maybe you can give it a spin on your end just to make sure. Unfortunately there's still a little bit of broken styling: when you explicitly set dark or light style in the extension settings and then switch between explicit dark or light theme in kagi the font color of bookmark descriptions change. There should be no changes in colours when an explicit theme is set for the injection box. It's still usable and I couldn't figure out a way to make it work today, so I will leave it as is for now. If you are feeling motivated, you can try your hand. Otherwise I will merge this as is since I don't think anyone actually uses the explicit styles anyway. |
@@ -165,9 +163,10 @@ div#bookmark-list-container { | |||
&.dark, | |||
.dark-bg &:not(.light), // DuckDuckGo | |||
body[data-dt="1"] &:not(.light), // Google | |||
:root:not(.light) &:not(.light), // Brave Search |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The brave search selector was actually too general and messing with the kagi ones. Without this automatic theme detection wasn't working properly for me. Just FYI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Makes sense! ANd yeah, the default theme was just something I missed in my original commit, but was in my system locally
Yeah I noticed this. I think its at least related to the fact that kagi has some styles specifically attached to OS-level dark mode that ignores the website-level decision. Kinda weird. IMO good enough to push as it, and if anyone other than me ends up using this and requests a fix I can look back at trying to solve it 😄 |
That's reasonable! Happy to merge, thank you again, will go live in the next release |
Just a small update to support the privacy-focused search engine, Kagi!