Use interactive buttons to display and send (toggle) reactions #168

alanpoon · 2024-10-01T15:05:05Z

Fixes #115
Fixes #115

Screen.Recording.2024-10-01.224004.mp4

Uploading wrap.mp4…

Clicking the button sends toggle_reaction
Display a row of buttons for interactions with wrap

kevinaboos

thanks, much appreciated!

please follow whitespace conventions, as it makes the code more readable. I added comments to some of the places where you're not doing it, but there are many more in room_reaction_list.rs.

src/sliding_sync.rs

src/home/room_screen.rs

src/home/room_reaction_list.rs

src/home/room_screen.rs

kevinaboos

Thanks Alan, things are looking pretty good now!

There is still a key feature that we need, which seems to be missing (?): two different colors for buttons.

one color for emphasis, which will be used for the buttons that represent reactions the current user has sent themselves.
a different color for non-emphasis, which will be used for buttons that represent reactions the user has not sent, i.e., reactions from others.

Also, can you provide a demo video / test case of this PR properly supporting wrapping the list of reaction buttons to a new line? I see that implemented in the ReactionList::draw_walk() fn but it has historically been difficult to implement properly.

Another thought I had was: can we reuse an existing Makepad widget instead of trying to roll our own list of buttons for the ReactionList? Seems like this should be a fundamental widget type, since it's just a set of buttons that wrap.
If not, we should discuss this with Rik/Julian to see if your work here could be contributed to the upstream Makepad repo, since i'm sure others would want to use your code here for a wrappable list of buttons.

src/home/event_reaction.rs

src/home/room_screen.rs

src/sliding_sync.rs

src/home/room_screen.rs

src/home/event_reaction.rs

alanpoon · 2024-10-30T06:36:54Z

Thanks Alan, things are looking pretty good now!

There is still a key feature that we need, which seems to be missing (?): two different colors for buttons.

one color for emphasis, which will be used for the buttons that represent reactions the current user has sent themselves.

a different color for non-emphasis, which will be used for buttons that represent reactions the user has not sent, i.e., reactions from others.

Also, can you provide a demo video / test case of this PR properly supporting wrapping the list of reaction buttons to a new line? I see that implemented in the ReactionList::draw_walk() fn but it has historically been difficult to implement properly.

Another thought I had was: can we reuse an existing Makepad widget instead of trying to roll our own list of buttons for the ReactionList? Seems like this should be a fundamental widget type, since it's just a set of buttons that wrap. If not, we should discuss this with Rik/Julian to see if your work here could be contributed to the upstream Makepad repo, since i'm sure others would want to use your code here for a wrappable list of buttons.

Here is the clip that features the wrap
https://github.com/user-attachments/assets/e55f576b-7fe7-43e8-b02a-457e8768b12e

kevinaboos · 2024-10-30T08:14:49Z

Here is the clip that features the wrap
https://github.com/user-attachments/assets/e55f576b-7fe7-43e8-b02a-457e8768b12e

awesome, very nice work!

can you add some more left & right padding to each button? maybe 5 instead of 3? Up to you, but it feels a bit tight to me.

alanpoon · 2024-10-30T08:29:04Z

Here is the clip that features the wrap
https://github.com/user-attachments/assets/e55f576b-7fe7-43e8-b02a-457e8768b12e

awesome, very nice work!

can you add some more left & right padding to each button? maybe 5 instead of 3? Up to you, but it feels a bit tight to me.

ok, thanks for the feedback. I might be making a generic wrap list button widget that is more composable.

kevinaboos · 2024-10-31T18:04:20Z

ok, thanks for the feedback. I might be making a generic wrap list button widget that is more composable.

Sounds good. but kindly save that for later. For now, all you need to do is address the above comments and then we can merge this in as-is. Thanks!

src/home/event_reaction.rs

kevinaboos · 2024-11-07T03:31:03Z

Blocked on Makepad problem: #168 (comment)

alanpoon · 2025-01-16T08:14:43Z

PR #324 fixed the issue with hits not being forwarded properly. Hopefully you don't have any more issues.

So, now you should be able to undo all the hit-related changes, and we can merge this in.

Merged in main.

kevinaboos · 2025-01-16T20:30:07Z

There are quite a few UI & UX issues still remaining. Have you tested this thoroughly? If not, please do so, since there are likely even more issues lurking under the surface.

Screenshot showing some issues:

Some of the problems are:

~~I also observe the half-sized RoomScreen that Julian saw.~~ Edit: I fixed this in Display read receipts next to each message as a row of Avatars for users who have seen that message #162
The color contrast for the reaction button, both the green color and the gray color, is really low/bad. It's nearly impossible to read.
I can't tell when a reaction has been sent by a user. Please follow the same color/shading styles used by Element or Discord, they make it quite clear whether or not you have sent a reaction.
Each reaction button needs to set its cursor to Hand such that the user is aware they can click on it. Right now, there's no way for the user to know that each reaction button is a clickable element.
Lots of emoji are failing to be parsed. I'm not sure what you changed about my routine that parses the raw emoji into a text shortcode, but mine previously worked 100%, so you may just want to revert back to what I was doing previously.
- Examples:
```
src/home/event_reaction_list.rs:186:21 - Failed to parse emoji: 👍️
src/home/event_reaction_list.rs:186:21 - Failed to parse emoji: ➕️
```

The tooltip is nice, but is split across too many lines, making it hard to read. It should only be split into two lines by default (with the first line being "UserName reacted with:"), and should only spill over to more than 2 lines if the text is really really long.

…the right

alanpoon · 2025-01-19T16:34:00Z

There are quite a few UI & UX issues still remaining. Have you tested this thoroughly? If not, please do so, since there are likely even more issues lurking under the surface.

Screenshot showing some issues:

Some of the problems are:
~~I also observe the half-sized RoomScreen that Julian saw.~~ Edit: I fixed this in Display read receipts next to each message as a row of Avatars for users who have seen that message #162

The color contrast for the reaction button, both the green color and the gray color, is really low/bad. It's nearly impossible to read.

I can't tell when a reaction has been sent by a user. Please follow the same color/shading styles used by Element or Discord, they make it quite clear whether or not you have sent a reaction.

Each reaction button needs to set its cursor to Hand such that the user is aware they can click on it. Right now, there's no way for the user to know that each reaction button is a clickable element.
Lots of emoji are failing to be parsed. I'm not sure what you changed about my routine that parses the raw emoji into a text shortcode, but mine previously worked 100%, so you may just want to revert back to what I was doing previously.

Examples:
src/home/event_reaction_list.rs:186:21 - Failed to parse emoji: 👍️
src/home/event_reaction_list.rs:186:21 - Failed to parse emoji: ➕️
The tooltip is nice, but is split across too many lines, making it hard to read. It should only be split into two lines by default (with the first line being "UserName reacted with:"), and should only spill over to more than 2 lines if the text is really really long.

Here is updated version.
https://github.com/user-attachments/assets/679ab04e-e14f-4c09-8b36-7b6eab774fe8

kevinaboos

Nice job redesigning the reaction buttons! They look very nice & professional now.

I also made a small commit of my own to fix the alignment of the tooltip as well as make the font size slightly larger.

There are a few very small things I'd like you to fix before I merge this in.

First, please make the callout arrow longer, maybe like 30-50% longer.

Second, the positioning of the callout triangle is a bit inconsistent. I think the easiest way to fix this is to make it vertically centered regardless of how many lines of text there are in the tooltip, but if you don't like that design then we can discuss other options, such as aligning the triangle vertically with the first (or last) line of text inside of the tooltip.

Here's an example of what I mean. It looks perfect for a 2-line tooltip, but it looks kind of bad for a 1-line tooltip or a 3-line tooltip:

src/app.rs

src/home/room_screen.rs

alanpoon · 2025-01-20T11:41:32Z

Nice job redesigning the reaction buttons! They look very nice & professional now.

I also made a small commit of my own to fix the alignment of the tooltip as well as make the font size slightly larger.

There are a few very small things I'd like you to fix before I merge this in.

First, please make the callout arrow longer, maybe like 30-50% longer.

Second, the positioning of the callout triangle is a bit inconsistent. I think the easiest way to fix this is to make it vertically centered regardless of how many lines of text there are in the tooltip, but if you don't like that design then we can discuss other options, such as aligning the triangle vertically with the first (or last) line of text inside of the tooltip.

Here's an example of what I mean. It looks perfect for a 2-line tooltip, but it looks kind of bad for a 1-line tooltip or a 3-line tooltip:

Align the callout to the first line of the tooltip
Enlarge the callout arrow
https://github.com/user-attachments/assets/8c360b29-bb49-4310-a53e-2f48412b84c5

kevinaboos

Excellent work, looks great! Thanks very much.

outdated

alanpoon added 5 commits September 27, 2024 19:07

emoji

1453a13

emo

5d13dfc

reaction_list

d77281a

added emoji toggle

20b4f0d

minor cleanup

ad3b14d

kevinaboos self-requested a review October 1, 2024 19:30

kevinaboos requested changes Oct 1, 2024

View reviewed changes

alanpoon added 9 commits October 2, 2024 14:44

Merge branch 'main' into interactive_icon_button_#115

632eaa7

removed message annotation template

0d0dc06

Merge branch 'main' into interactive_icon_button_#115

c26719f

resolve conflict

f9bbb9f

commiting cargo.lock

39c7c58

revert use main's cargo.lock

4f818a3

did some formating

3f5500c

Merge branch 'main' into interactive_icon_button_#115

d3a0b29

resolve minor issue

0db2482

alanpoon requested a review from kevinaboos October 11, 2024 03:09

alanpoon added 3 commits October 12, 2024 14:53

Merge branch 'main' into interactive_icon_button_#115

926871d

resolve conflict

e585205

merge main n resolve conflict

0279b2e

kevinaboos requested changes Oct 30, 2024

View reviewed changes

Merge branch 'main' into interactive_icon_button_#115

b02c4ce

alanpoon commented Oct 31, 2024

View reviewed changes

src/home/event_reaction.rs Outdated Show resolved Hide resolved

kevinaboos added blocked Blocked on another issue or missing feature and removed blocked Blocked on another issue or missing feature labels Nov 7, 2024

kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Jan 16, 2025

kevinaboos mentioned this pull request Jan 16, 2025

Display read receipts next to each message as a row of Avatars for users who have seen that message #162

Merged

alanpoon added 4 commits January 17, 2025 18:53

Fix half screening reaction button styling

0a3118b

improving tooltip clearing

b5f2b93

Merge branch 'main' into interactive_icon_button_#115

b39631f

fix edge toggle reaction

05e7d0e

Display tooltip at the bottom if the reaction button is too close to …

f89ef19

…the right

alanpoon added waiting-on-review This issue is waiting to be reviewed and removed waiting-on-author This issue is waiting on the original author for a response labels Jan 19, 2025

kevinaboos requested changes Jan 20, 2025

View reviewed changes

src/app.rs Show resolved Hide resolved

src/home/room_screen.rs Outdated Show resolved Hide resolved

kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Jan 20, 2025

align callout to the first line

0229149

alanpoon added waiting-on-review This issue is waiting to be reviewed and removed waiting-on-author This issue is waiting on the original author for a response labels Jan 20, 2025

kevinaboos and others added 2 commits January 20, 2025 11:17

Enlarge reaction font size. Left-align reaction buttons.

0fd23c3

Merge branch 'main' into interactive_icon_button_#115

6189c44

kevinaboos removed the waiting-on-review This issue is waiting to be reviewed label Jan 20, 2025

kevinaboos approved these changes Jan 20, 2025

View reviewed changes

kevinaboos merged commit e521d8d into project-robius:main Jan 20, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use interactive buttons to display and send (toggle) reactions #168

Use interactive buttons to display and send (toggle) reactions #168

alanpoon commented Oct 1, 2024 •

edited

Loading

kevinaboos left a comment

kevinaboos left a comment

alanpoon commented Oct 30, 2024

kevinaboos commented Oct 30, 2024

alanpoon commented Oct 30, 2024

kevinaboos commented Oct 31, 2024

kevinaboos commented Nov 7, 2024

alanpoon commented Jan 16, 2025

kevinaboos commented Jan 16, 2025 •

edited

Loading

alanpoon commented Jan 19, 2025

kevinaboos left a comment •

edited

Loading

alanpoon commented Jan 20, 2025

kevinaboos left a comment

Use interactive buttons to display and send (toggle) reactions #168

Use interactive buttons to display and send (toggle) reactions #168

Conversation

alanpoon commented Oct 1, 2024 • edited Loading

kevinaboos left a comment

Choose a reason for hiding this comment

kevinaboos left a comment

Choose a reason for hiding this comment

alanpoon commented Oct 30, 2024

kevinaboos commented Oct 30, 2024

alanpoon commented Oct 30, 2024

kevinaboos commented Oct 31, 2024

kevinaboos commented Nov 7, 2024

alanpoon commented Jan 16, 2025

kevinaboos commented Jan 16, 2025 • edited Loading

alanpoon commented Jan 19, 2025

kevinaboos left a comment • edited Loading

Choose a reason for hiding this comment

alanpoon commented Jan 20, 2025

kevinaboos left a comment

Choose a reason for hiding this comment

alanpoon commented Oct 1, 2024 •

edited

Loading

kevinaboos commented Jan 16, 2025 •

edited

Loading

kevinaboos left a comment •

edited

Loading