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

Add "enter" hint to in-gameplay chatbox placeholder text #29281

Merged
merged 6 commits into from
Aug 7, 2024

Conversation

kstefanowicz
Copy link
Contributor

@kstefanowicz kstefanowicz commented Aug 4, 2024

I don't know how to get to a multiplayer game in a debug build, so I was not able to confirm if this works.

@smoogipoo
Copy link
Contributor

I'm not sure about:

  1. The length of the placeholder in general. It feels too verbose but I'm not sure how to improve it.
  2. The placeholder still saying "press enter [...]" while it already has focus.

Regarding the second from the above, I experimented with something like this:

diff --git a/osu.Game/Online/Chat/StandAloneChatDisplay.cs b/osu.Game/Online/Chat/StandAloneChatDisplay.cs
index 3a094cc074..e100b5fe5b 100644
--- a/osu.Game/Online/Chat/StandAloneChatDisplay.cs
+++ b/osu.Game/Online/Chat/StandAloneChatDisplay.cs
@@ -128,6 +128,9 @@ private void channelChanged(ValueChangedEvent<Channel> e)
 
         public partial class ChatTextBox : HistoryTextBox
         {
+            public Action Focus;
+            public Action FocusLost;
+
             protected override bool OnKeyDown(KeyDownEvent e)
             {
                 // Chat text boxes are generally used in places where they retain focus, but shouldn't block interaction with other
@@ -153,13 +156,17 @@ protected override void LoadComplete()
                 BackgroundFocused = new Color4(10, 10, 10, 255);
             }
 
+            protected override void OnFocus(FocusEvent e)
+            {
+                base.OnFocus(e);
+                Focus?.Invoke();
+            }
+
             protected override void OnFocusLost(FocusLostEvent e)
             {
                 base.OnFocusLost(e);
                 FocusLost?.Invoke();
             }
-
-            public Action FocusLost;
         }
 
         public partial class StandAloneDrawableChannel : DrawableChannel
diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs
index 656071ad43..d1a73457e3 100644
--- a/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs
+++ b/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs
@@ -42,8 +42,13 @@ public GameplayChatDisplay(Room room)
 
             Background.Alpha = 0.2f;
 
-            TextBox.FocusLost = () => expandedFromTextBoxFocus.Value = false;
             TextBox.PlaceholderText = ChatStrings.InGameInputPlaceholder;
+            TextBox.Focus = () => TextBox.PlaceholderText = Resources.Localisation.Web.ChatStrings.InputPlaceholder;
+            TextBox.FocusLost = () =>
+            {
+                TextBox.PlaceholderText = ChatStrings.InGameInputPlaceholder;
+                expandedFromTextBoxFocus.Value = false;
+            };
         }
 
         protected override bool OnHover(HoverEvent e) => true; // use UI mouse cursor.
Screen.Recording.2024-08-05.at.12.37.08.mov

But I'm still not too sure about it. Need a secondary opinion.

@frenzibyte
Copy link
Member

"press enter to chat..." could work better as a shorter placeholder.

@kstefanowicz
Copy link
Contributor Author

kstefanowicz commented Aug 5, 2024

  1. The length of the placeholder in general. It feels too verbose but I'm not sure how to improve it.

"press enter to chat..." is definitely better, I will change that now

  1. The placeholder still saying "press enter [...]" while it already has focus.

That implementation makes sense, agreed. I believe that multiplayer in-game is the only place where the chatbox isn't in focus at all times, so the Focus events may be able to be applied to just GameplayChatDisplay and not the parent StandAloneChatDisplay edit - I'm bad at reading code, thank you for your patience

@peppy
Copy link
Member

peppy commented Aug 7, 2024

@kstefanowicz are you going to apply the proposed change to the placeholder text when focused?

@pull-request-size pull-request-size bot added size/S and removed size/XS labels Aug 7, 2024
@kstefanowicz
Copy link
Contributor Author

Placeholder text hover change has been applied. Thank you for your patience.

smoogipoo
smoogipoo previously approved these changes Aug 7, 2024
@peppy peppy self-requested a review August 7, 2024 15:44
@peppy peppy merged commit 8773c2f into ppy:master Aug 7, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hint to press enter key for focusing in-gameplay chat
4 participants