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

Implements "find" tag for webbrowsers #1405

Merged
merged 8 commits into from
Nov 9, 2024

Conversation

maxbruening
Copy link
Contributor

@maxbruening maxbruening commented Mar 20, 2024

The goal of this PR is to add basic search functionality for webbrowsers based on the new "find" tag from the other already merged PR 1404. The code has been tested on windows, but not on mac or Linux.

I left a commented out version for def find() in the PR as this alternative version may be more robust.

I checked and tested that the implemented keyboard actions work in windows for chrome, firefox, opera, edge, and brave.

@maxbruening maxbruening changed the title Implements new "find" tag for webbrowsers Implements "find" tag for webbrowsers Jun 5, 2024
Copy link
Collaborator

@AndreasArvidsson AndreasArvidsson left a comment

Choose a reason for hiding this comment

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

I don't think there are any shortcuts that are general enough for all browsers. The shortcuts are gonna be different depending on platform and application and should be defined in the application context. We should also not use user.find when there is edit.find. We should definitely just migrate everything to the edit actions.

@AndreasArvidsson
Copy link
Collaborator

Here is a pull request to replace user.find with edit.find
#1499

@maxbruening
Copy link
Contributor Author

Alright, happy to implement those changes.
Before I do so: Are we sure that we want to implement the python functions on a per browser basis? The keyboard presses are the same for chrome, firefox, opera, edge, and brave. Given this, wouldn't it be simpler to define those functions once and then if another browser has different shortcuts, the python file for that browser should redefine them?

@AndreasArvidsson
Copy link
Collaborator

Most of the work is already done in #1499
I don't think we should put any key bindings under browser it's probably operating system specific in that case. In my pr above I have already added the default operating system shortcuts.

@maxbruening
Copy link
Contributor Author

browser_shortcuts

tags/browser/browser.py Outdated Show resolved Hide resolved
tags/browser/browser_mac.py Outdated Show resolved Hide resolved
@knausj85
Copy link
Member

knausj85 commented Nov 2, 2024

From a quick test on a number of browsers on both windows and macOS, I believe all we need to do here is enable the tag in browser.talon. Re-requested review for sanity

@AndreasArvidsson AndreasArvidsson merged commit c555c2e into talonhub:main Nov 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants