search

prose-im / prose-app-macos

Prose macOS application. XMPP client for team messaging.
https://prose.org/downloads
Mozilla Public License 2.0
58 stars 3 forks source link

Message reaction picker #91

Closed RemiBardon closed 2 years ago

RemiBardon commented 2 years ago

https://user-images.githubusercontent.com/37386490/182117014-409f8e19-5058-4a94-8245-d72f09f5d7ba.mov

I tried to make things clean, but the only approach I found was to use a tiny text field. I made sure we only create one and remove it whenever we can, so the web view doesn't have dozens of subviews.

Another thing: I started by developing the picker in SwiftUI, but when I tried to integrate it here, I couldn't put the focus on the NSHostingView for the popup to show in the right location.

It was easy to replicate it with AppKit, so I kept it that way. It's not something we will have to port to UIKit, so it's fine (I guess).

RemiBardon commented 2 years ago

For some reason, showing the emoji picker multiple times worked before, but now on the second time it shows as a detached window. Let me mark the PR as draft while I fix this…

RemiBardon commented 2 years ago

Ok I got it: I was reusing the text field, but not resetting the text. After one emoji was inserted, there would not be enough space for another, and the picker would not know where to attach the popup.

By the way, not resetting the text would also cause a pixel to be colored to the color of the emoji, so I fixed two problems at the same time.

valeriansaliou commented 2 years ago

Good, just two comments:

nesium commented 2 years ago

Good catch, regarding the delay. It seems to only happen for the first click. Maybe we can pre-warm (read: pre-initialize) the view if the delay originates there.

Do we actually have the frame of the button? I believe we only receive a point for the click?

RemiBardon commented 2 years ago

For the placement, yes I only have the coordinates of the click. I could offset it but it would not look good IMO if the user clicks at the bottom of the button.

For the delay, I'm not sure I get what you're talking about, but I'll try again soon to see if I get it too. The only delay I had seen is the delay between when the character palette popover shows and when the view gets populated (the popover opens with no content, an after a brief delay the emojis appear). However, this seems to be macOS internal behavior, as I had the same results using the picker in other system text fields.

nesium commented 2 years ago

Funnily I cannot reproduce it anymore. It seemed that there was a delay on first click so I was speculating that the hidden textfield would need to be attached and possibly layouted. Could be a problem with the picker itself though, OS-wise.

I've noticed however - and that could also be the perceived reason - that in the middle of the smiley you cannot actually press it. Note how the cursor changes to an I-Beam…

https://user-images.githubusercontent.com/39174/182372637-9855d3c1-dc80-4f00-b303-7e38d97312ba.mov

Edit: Oh, that's because of the textfield. I guess we should hide that when the picker was dismissed.

RemiBardon commented 2 years ago

I was not able to remove the text field on dismiss, I only remove it when the user selects an emoji. But as you proposed, with NSTextInputClient I could fix this issue.

RemiBardon commented 2 years ago

macOS doesn't allow me to know when the character palette is closed, so sometimes we have a state inconsistency. We could close the picker and clean the state every time an emoji is entered, but this would prevent the user from entering multiple reactions when the picker is a standalone window (not a popover). This inconsistency adds a small delay when adding multiple reactions one after another, either because we open the message menu (right click or ellispis button) or because we have to click twice on the emoji shortcut button. It's only 0.6 second, so it's good enough in terms of UX. In prose-core-views, we will add the opportunity to toggle the state of the emoji shortcut button so the user sees the inconsistency, and it will improve this UX until macOS provides a better API.

I could also try to open the character palette as a standalone window whe the user adds two reactions in a row, that could be great. If it's simple enough, I will add it to this PR.

RemiBardon commented 2 years ago

@nesium I just rebased, but I suppose you want to merge #94 first, which fixes make preflight, so I can run it here?

RemiBardon commented 2 years ago

Do you want me to make the emoji picker a standalone view? I was planning on doing it when adding support for the emoji button in the message bar.

RemiBardon commented 2 years ago

After a team discussion, we decided to build a custom emoji picker, until AppKit provides a better and more open API. This will ensure stability and consistency across platforms. I will close this PR and #92, and start a new PR for the new picker.