Rework the hotkeys used to switch between tabs
AcceptedPublic

Authored by weabot on Sep 29 2017, 4:29 AM.

Details

Reviewers
hein
Group Reviewers
Konversation
Summary

The hotkeys were reworked for Alt+1..0 to switch between the channels specifically to the current server we're under and Alt+Shift+1..0 to switch between only the servers. This should make it easier and (in my opinion) more intuitive to handle connections to many channels at once over many networks, as tends to happen when you go through a bouncer, which are increasingly popular.

Test Plan

An issue was found throughout the process of adding this where Alt+1 and Alt+0 returned an "ambiguous shortcut" error message from Qt. I found out that the weird pattern of it only being Alt+1 and Alt+0 was pure coincidence. I am in two channels, one of which starts with a 0, the other with a 1. The shortcuts Alt+1 and Alt+0 were therefore given to them. I'll start working on this issue.

There was another potential issue that I haven't experienced where if a user would disconnect and close a server, then join another server, the server list wouldn't be updated because the size would still be the same. The hotkey would try to go to a non-existant ChatWindow* and lead to unexpected things. I decided to simply unconditionally update the server list on every call as a lazy workaround.

So as of right now, there should be no introduced bugs as a result of this patch.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
weabot created this revision.Sep 29 2017, 4:29 AM
weabot updated this revision to Diff 20075.Sep 29 2017, 4:44 AM

Just fixed some style issues

weabot updated this revision to Diff 20117.Sep 29 2017, 8:38 PM
weabot edited the test plan for this revision. (Show Details)
weabot updated this revision to Diff 20164.EditedSep 30 2017, 5:18 PM
weabot edited the test plan for this revision. (Show Details)
  • Some style tweaks in the mainwindow.cpp loops
  • Removed the condition required to update m_serverChatWindowList because it could introduce an issue and more expensive checks probably wouldn't be worth doing for a list this small
weabot updated this revision to Diff 20166.Sep 30 2017, 5:25 PM
hein accepted this revision.Oct 2 2017, 6:20 AM
hein added a subscriber: hein.

I like it!

This revision is now accepted and ready to land.Oct 2 2017, 6:20 AM
weabot added a comment.Oct 3 2017, 1:32 AM
In D8053#151458, @hein wrote:

I like it!

Thank you!

ngraham added a subscriber: ngraham.Oct 3 2017, 1:33 AM
hein added a comment.Oct 3 2017, 7:38 AM

Do you have developer access or would you like me to push the patch for you?

Note we do really prefer real names on commits. Did you intend to stay anonymous or would you like your contribution properly attributed ...?

weabot added a comment.EditedOct 3 2017, 7:21 PM
In D8053#151811, @hein wrote:

Do you have developer access or would you like me to push the patch for you?

Note we do really prefer real names on commits. Did you intend to stay anonymous or would you like your contribution properly attributed ...?

I don't have developer access, so it would be appreciated. I would like to stay anonymous for now, but I'd be willing to give away any copyright rights to the contribution (the diff and all of its versions in this repository) if that's the issue, or to directly give every right over it to the KDE project if that's possible. I understand that it might be crippling to have anonymous contributors who can't be contacted but still own rights if any legal change were to be necessary.

I'll keep it in mind and settle this for any future contribution.

Thank you!