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

Allow macOS to use CMD+R to refresh repositories #13

Closed
wants to merge 1 commit into from

Conversation

ProjectInfinity
Copy link
Contributor

On macOS it is pretty common to use Command + R to refresh views as opposed to F5. This change does not remove the ability to use F5 but it adds a case for if the platform is macOS to use Command + R instead.

@ProjectInfinity ProjectInfinity mentioned this pull request Jun 29, 2022
@JetpackDuba
Copy link
Owner

I like the idea but not the execution. I've also seen also Ctrl+R for Linux and, less often, for Windows. In other words, to cover all the cases it should be a single method that checks for any refresh combination (depending on the platform). Probably a function (even an extension function) that checks if it's the right combination for the right platform.

However, thinking a bit more about the itself, the problem is not exclusive of the refresh, it's also present in the quick commit (ctrl+enter) and a few dialogs if my memory does not fail.

So I'd like to fix all the cases where ctrl/cmd rather than apply a patch specific for this case. If you want to update the PR doing so it'd be great. Otherwise I'll change it myself at some point (probably soon).

Doing a quick overview, I'd create an object class that contains a lazy property with the current OS and one method for each possible combination.

@JetpackDuba
Copy link
Owner

Thinking about it a bit more, ignore my suggestion of how to do it. I think that creating a map of a new data class for each platform would be a better idea, since it would allow us to also directly map each combination with its corresponding feature and display these shortcuts as text tooltips.

The concept is similar but the OS is checked once. Whenever a key event is fired, it should be compared with the entries from the dictionary rather than verifying each combination. It may get quite more complex than I was anticipating since it will require to change multiple components (Aside from adding the tooltips to each button).

Let me know what do you think.

@ProjectInfinity
Copy link
Contributor Author

I think the approach described in your last comment sounds good, but I'm not sure how it would look in terms of implementation. I think that it is best that you do an initial implementation which I could expand on in the future.

Feel free to close the PR if you would rather do that first.

JetpackDuba added a commit that referenced this pull request Jul 22, 2022
This feature allows to easily add new keybindings (programmatically) to features accross the app.

Should also fix the issue that #13 tried to solve
@JetpackDuba
Copy link
Owner

A bit late but I've implemented what I mentioned before. It needs some polishing but should work just fine.

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.

None yet

2 participants