Focus on composition area when pressing a key - #4998
Conversation
|
Currently (1.40.0-beta.4), closing the reply dialogue loses the focus on composition area, would this prevent that too? |
|
@xashyar Well, the focus is still lost. However, you can still continue to type and the composition area gains focus automatically (also no characters are lost). So, whether it fixes it depends on your definition since the composition area still loses focus but it does not impact your typing. |
|
I think this PR would be really beneficial. Currently, I often have to click inside the text input area to be able to type, and it is getting slightly annoying. I'm not sure what causes the text input area to lose focus. I often just start typing and sometimes nothing appears, because focus is lost. |
|
@EvanHahn-Signal is there any plan on merging this? I guess quite a lot of people are unsatisfied with the current typing behavior. |
|
@n-gao sorry we marked this as "needs review" to keep stale bot away when we added stale bot. Not sure if we actually want to do this though, I mean it seems fine on the surface but:
We're also worried about the implementation, there will now be a global keydown listener and that'll need to be maintained, seems like it could be a recipe for potential disaster if any of the other pieces that it needs to short-circuit on change and this code isn't updated to reflect that. I'm happy to hear counterpoints to this -- I sometimes experience this issue and start typing but the composition area isn't in focus. I usually quickly realize and just focus it with cmd+shift+t |
|
cmd+shift+t seems undocumented to me. I wasn't aware it existed, and I cannot find the shortcut in the menu bar or in preferences. Now that I know of it: neat! I will test if this is sufficient for me. I sometimes find it surprising that the focus is not in the text input field, not sure why it is not sometimes. |
I wasn't aware of it either, but it does appear to be documented in both the ⌘+/ popup and the respective support article under the name "Focus composer". |
|
@josh-signal The comments by @u32i64 and @larstobi illustrate the difficulty of having to rely on hotkeys. Most people (including me) are not aware of these and for them, it is mostly an annoyance having to click on the composing box. Imho, hotkeys are always heavy user-oriented. Is having Signal outside of your screen and the composer focused an actual issue? Right now, I could also have been typing and then moving the window outside of my screen (not sure why anyone would do that) and the focus on the composer would stay. If the window is minimized it automatically loses focus (at least on windows but I would be surprised if other OS's keep the focus if you minimized a window) and you can't type. Furthermore, other chat clients like WhatsApp or Telegram decided it not being a real issue and include this feature. Regarding the global observer, I see your concerns. This is why it's not a global observer (like hotkeys) but restricted to the composition area. I think that the composing area is a good compromise since it's easy to hit without affecting parts of the application where it definitely should not trigger. If the focus is on the contact pane the focus obviously should not switch to the composer. |
|
Yeah the shortcuts aren't intuitive which is why this PR exists. edit: But glad ya'll are aware of them now 😄
In most cases I'm guessing no, but it is a possibility so I raised it as a concern.
It's adding the event listener to |
| } | ||
|
|
||
| // We don't want to switch focus if another panel is up | ||
| const panels = document.querySelectorAll('.conversation .panel'); |
There was a problem hiding this comment.
Also these kind of class lookups are brittle, if we ever change conversation | panel this would fall apart. I think ideally redux would know about panel state and we can just query that selector if there are panels active or not.
There was a problem hiding this comment.
Is this information already saved in a Redux store somewhere? Or are you implying that it should be added? (And if you're suggesting it be added, where would be your suggestion for where to add this?)
There was a problem hiding this comment.
Also, seems like this already has precedent in the codebase:
Signal-Desktop/ts/components/stickers/StickerButton.tsx
Lines 193 to 196 in 6700d2f
Maybe refactoring this could be done in a different PR?
|
@n-gao do you think that you will be able to get back to this sometime soon? This is a feature that every other desktop messenger client has, so it's a big UX drawback that this doesn't already exist. |
| return; | ||
| } | ||
|
|
||
| // We don't want to take focus away of input fields |
There was a problem hiding this comment.
Clarity:
| // We don't want to take focus away of input fields | |
| // Avoid stealing focus from focused input fields |
| const { activeElement } = document; | ||
| if (activeElement?.nodeName.toLowerCase() === 'input') { |
There was a problem hiding this comment.
Style: Avoid extra identifier in scope
| const { activeElement } = document; | |
| if (activeElement?.nodeName.toLowerCase() === 'input') { | |
| if (document.activeElement?.nodeName.toLowerCase() === 'input') { |
| }; | ||
| }, [setLarge]); | ||
|
|
||
| // Listen to any key in the conversation panel to |
There was a problem hiding this comment.
| // Listen to any key in the conversation panel to | |
| // Focus message composition input when key is pressed |
| return; | ||
| } | ||
|
|
||
| inputApiRef.current?.focus(); |
There was a problem hiding this comment.
Should this use the focusInput callback instead?
If yes, then:
| inputApiRef.current?.focus(); | |
| focusInput(); |
| document.addEventListener('keydown', handler); | ||
| return () => { | ||
| document.removeEventListener('keydown', handler); | ||
| }; |
There was a problem hiding this comment.
Surfacing @josh-signal's comment here in a conversation:
It's adding the event listener to
documentso yeah it's grabbing all input. Perhaps abettersafer approach would be to figure out what node is focused and listen tokeypresson that node? I think we can test the "in focus" issue using something like macOS' spaces.
There was a problem hiding this comment.
@josh-signal to be clear, you're suggesting this change?
| document.addEventListener('keydown', handler); | |
| return () => { | |
| document.removeEventListener('keydown', handler); | |
| }; | |
| document.activeElement.addEventListener('keypress', handler); | |
| return () => { | |
| document.activeElement.removeEventListener('keypress', handler); | |
| }; |
Not sure if this will work, but at least it's a starting place for the conversation.
| }, [setLarge]); | ||
|
|
||
| // Listen to any key in the conversation panel to | ||
| React.useEffect(() => { |
There was a problem hiding this comment.
useEffect has now been imported as a named import
| React.useEffect(() => { | |
| useEffect(() => { |
|
For anyone else who doesn't want to wait for this to be accepted, I've created a patcher for the Signal desktop Electron app (macOS only for now): https://github.com/karlhorky/electron-app-patcher Just clone, install the dependencies and run Edit: Upgraded my patch for Signal 6.0.0: karlhorky/electron-app-patcher@6fbdffe |
|
@josh-signal or @jamiebuilds-signal would it be possible to get another review on this? Would love to get a version of this missing UX feature in the app finally. |
|
why is this no merged already?? |
|
I updated my patch for Signal 6.3.0, which was released recently: |
|
No other patch changes have been necessary for the new versions up to Signal 6.9.0, although it is annoying to have to update every time an update comes out:
|
|
@indutny-signal any news on what it would take to get this merged? Or a timeline? |
Signal Patch ChangelogVersion changelog for all of the fixes needed with the new versions of Signal:
Hopefully we can get this missing UX feature sometime soon, so that I don't need to keep maintaining this. |
|
@scottnonnenberg-signal @indutny-signal @josh-signal friendly ping - would love to be able to get this missing feature in Signal. Could someone from the Signal team respond here? |
|
what's the major factor blocking this pr |
|
Having this feature would really help a lot. |
|
Yearly nudge - what's blocking this? As it stands this is my one big problem with using Signal desktop as I frequently tab around my system and having to either hit a keybind or reach for my mouse to focus the input field slows me down to the point of wanting to use another app just for speed. Several other instant messaging platforms adopt this behaviour and it's kinda come to be expected as default. |
|
Yeah, I think this issue needs more publicity. As of now, I think the most common triggers of the broken behavior, eg:
...for most users will just lead to a weird feeling of discomfort, as if Signal desktop doesn't work properly. And most users won't know where to look or report the problem. So a (new) central public GitHub issue which we can direct a lot of community attention and publicity to would be a good start, I think. The Signal team hasn't prioritized it yet, but hopefully seeing a large amount of their userbase pushing for a public issue (instead of it getting lost and ignored in the PRs here) would cause them to change this stance on fixing this important UX bug. |
|
There was #1096 tracking this prior to the community forums, but that's now been moved to https://community.signalusers.org/t/desktop-focus-to-input-field-on-window-click/8930, there's also https://community.signalusers.org/t/allow-typing-when-chatbox-isnt-focused/62163. Unfortunately neither of these have seen much movement either - so it'd probably be best to push for comments there + general social platforms. Really hope to see this change land sooner rather than later, it's been 8 years since the original request was made for something I'd argue is rather simple (and even could just be a toggle). |
|
Looks like more users @AlterNezKo @performancer and @SteveDinn reporting the same problem over in a more recent issue (Aug 2024, Sep 2024): (issue was closed by @jamiebuilds-signal for a reason unrelated to the original issue description) There appears to be already a very high number of users who have reported this, across the various resources we've seen so far. And judging by typical open source contribution patterns, many more than that who are affected and do not like it but do not know where to report. |
|
Bumping the issue. It's been open for 5 years and still nothing! What could be preventing merge? |
|
The thing that is preventing merge are a deep look at possible edge cases that are related to this feature and adding tests, so that this doesn't turn into a long tail of bug reports after it's added.
And also, in another PR from 2018 (one other similar focus issue reported in 2017, since the focus bugs have been going on for a long time):
So it looks like the Signal team:
If anyone would have the possibility of doing a deep dive (in a new PR / issue) on that, to fix this broken UX between Signal desktop and pretty much every other messenger, then I will try to support with testing! |
Add auto-focus behavior that moves focus to the message composition input when a printable key is pressed, matching the UX of other major desktop messengers. Includes a user-facing toggle in Settings > Chats (default: on) and comprehensive guards against focus stealing from other inputs, keyboard shortcut interference, and non-printable keys. Supersedes signalapp#4998.
|
@karlhorky Hi, I've decided to give this PR a fresh shot. Seems this one has merge conflicts and is dead. The new PR should fix most of the original criticisms of privacy and not using idiomatic calls. Link: #7722 |
13 participants
First time contributor checklist:
Contributor checklist:
developmentbranchyarn readyrun passes successfully (more about tests here)Description
At the moment, a user has to specifically focus on the composition field to write a message. However, it is often more convenient to just click somewhere into the application and start typing with the application automatically moving the focus.
Other chat applications do this too.
So, this PR enables that. The focus is only moved for printable characters and not for control characters and does not move the focus if another input field is currently selected.
Demo:
