Chat up and down arrow key scrolls over chat channel message history #3059
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #3059 +/- ##
=============================================
- Coverage 58.98% 58.90% -0.08%
+ Complexity 4458 4455 -3
=============================================
Files 561 561
Lines 20322 20350 +28
Branches 1025 1033 +8
=============================================
+ Hits 11987 11988 +1
- Misses 7790 7813 +23
- Partials 545 549 +4
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Now that the state overhaul has been deployed you can use this with the chat messages kept in the chatChannel object once you rebase |
Kimo-s
force-pushed
the
feature/#2881-arrow-up-insert-latest-chat-message
branch
from
e81b1c8 to
65e94b7
Compare
src/main/java/com/faforever/client/chat/ChatUserItemController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/faforever/client/chat/ChannelTabController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/faforever/client/chat/ChannelTabController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
After testing there are a few things that should probably change to make this usable:
- There is no way currently to get back to a blank/in progress message
- You can easily lose the message you are typing by pressing the up or down arrow key accidentally
- when there is text it was taking two button presses to traverse the message history
| private String currentUserMessage = ""; | ||
| private int curMessageHistoryIndex = 0; |
There was a problem hiding this comment.
Move these down with the other private non final variable declarations
| @@ -431,6 +465,9 @@ private void sendAction(final TextInputControl messageTextField, final String te | |||
| } | |||
|
|
|||
| protected void onChatMessage(ChatMessage chatMessage) { | |||
| if(chatMessage.username().equals(playerService.getCurrentPlayer().getUsername())) { | |||
| userMessageHistory.add(chatMessage); | |||
There was a problem hiding this comment.
We should make sure to limit this to a max size. Lets just go with 50
| if(chatMessage.username().equals(playerService.getCurrentPlayer().getUsername())) { | ||
| if(userMessageHistory.size() >= 50){ | ||
| userMessageHistory.remove(0); | ||
| userMessageHistory.add(chatMessage); | ||
| } else { | ||
| userMessageHistory.add(chatMessage); | ||
| } | ||
| } |
There was a problem hiding this comment.
One thing I realized is that this is probably better off put in the send message method. Then you don't have to check the username
| if(event.getCode() == KeyCode.DOWN) | ||
| curMessageHistoryIndex--; | ||
| if(event.getCode() == KeyCode.UP) | ||
| curMessageHistoryIndex++; |
There was a problem hiding this comment.
Add brackets to the if statement
Sheikah45
force-pushed
the
feature/#2881-arrow-up-insert-latest-chat-message
branch
from
4abd7ff to
9a284d5
Compare
#2881