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

chat focus loss when room owner re-assignes sides #2817

Closed
vgaming opened this issue Apr 2, 2018 · 8 comments
Closed

chat focus loss when room owner re-assignes sides #2817

vgaming opened this issue Apr 2, 2018 · 8 comments
Labels
Bug Issues involving unexpected behavior. MP Lobby Issues with the multiplayer lobby UI and its components. Ready for testing Issues for which a potential fix is available but untested. UI User interface issues, including both back-end and front-end issues.

Comments

@vgaming
Copy link
Member

vgaming commented Apr 2, 2018

Steps:

  • create a game on MP server
  • join the game with another client (second client)
  • start typing anything in game chat with the second client (not room owner)
  • let first client (room owner) re-assign a side. Like choosing between "Network Player" and "Empty" and player names

What you'll observe at this point: chat focus on the second client gets lost. Focus goes elsewhere right while you typed something. (Thus breaking your typing.)

@vgaming vgaming added the Bug Issues involving unexpected behavior. label Apr 2, 2018
@Vultraz
Copy link
Member

Vultraz commented Apr 8, 2018

Honestly, this is expected. Most apps would do that.

@CelticMinstrel
Copy link
Member

Stealing focus is never expected and is always bad. The only reason most apps do it is because it's not actually an easy problem to solve. For this particular case it's about a text field focus, so... how do you determine whether the user is currently typing into the text field? How do you ensure that the text field remains focused? It may be inevitable that events temporarily remove the focus, so what you'd have to do is ensure it gets restored before keyboard event handling continues.

I think this is a good thing to fix, eventually.

@CelticMinstrel CelticMinstrel added the UI User interface issues, including both back-end and front-end issues. label Apr 8, 2018
@vgaming
Copy link
Member Author

vgaming commented Apr 8, 2018

@CelticMinstrel why not just change underlying value? Aren't these things orthogonal: UI focus and data?
I mean, you can chat in e.g. Facebook or an IM messenger. And no matter which notifications, messages, previews or pictures you get, your focus just remains where it was. The program doesn't have to change focus in order to change data right?

@Vultraz can you please give at least one well-known example of anything that would remotely alter focus on your client? Any web interface, or native, or anything.

@CelticMinstrel
Copy link
Member

Yeah, it shouldn't have to, so if it can be fixed in a way that doesn't remove the focus in the first place, that's even better.

@ProditorMagnus
Copy link
Contributor

Also happens when someone joins or leaves game.

@Wedge009 Wedge009 added the MP Lobby Issues with the multiplayer lobby UI and its components. label Oct 16, 2019
@jostephd
Copy link
Member

This fixes it, but makes it so it's not possible to use up arrow / down arrow to scroll the sides list. It also makes it so the chat box is focused when that dialog is entered.

diff --git a/src/gui/dialogs/multiplayer/mp_join_game.cpp b/src/gui/dialogs/multiplayer/mp_join_game.cpp
index 760f9b9eb23..e36c296ff97 100644
--- a/src/gui/dialogs/multiplayer/mp_join_game.cpp
+++ b/src/gui/dialogs/multiplayer/mp_join_game.cpp
@@ -339,7 +339,7 @@ void mp_join_game::generate_side_list(window& window)
 
 	tree_view& tree = find_widget<tree_view>(&window, "side_list", false);
 
-	window.keyboard_capture(&tree);
+	window.keyboard_capture(&find_widget<chatbox>(&window, "chat", false));
 
 	tree.clear();
 	team_tree_map_.clear();

@vgaming
Copy link
Member Author

vgaming commented Oct 18, 2019

I think that's pretty good. I am curious of course if we can insert some additional easy (yet maintainable) hack to detect up/down keystrokes and do the appropriate action, but even in the proposed form, I think it's worth it.

I would merge.

@jostephd jostephd self-assigned this Oct 19, 2019
@jostephd jostephd added the Ready for testing Issues for which a potential fix is available but untested. label Oct 19, 2019
@jostephd jostephd added this to the pre-1.16.0 string freeze milestone Oct 19, 2019
@jostephd jostephd removed their assignment Nov 4, 2019
@Pentarctagon Pentarctagon removed this from the pre-1.16.0 string freeze milestone Aug 10, 2020
@soliton-
Copy link
Member

soliton- commented Nov 1, 2020

Fixed with #5252.

@soliton- soliton- closed this as completed Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. MP Lobby Issues with the multiplayer lobby UI and its components. Ready for testing Issues for which a potential fix is available but untested. UI User interface issues, including both back-end and front-end issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants