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

Feature/40437 emoji reactions to work package comments #16020

Merged
merged 55 commits into from
Oct 25, 2024

Conversation

jjabari-op
Copy link
Collaborator

@jjabari-op jjabari-op commented Jul 2, 2024

Ticket

https://community.openproject.org/projects/openproject/work_packages/40437

What are you trying to accomplish?

Support emoji reactions to work package comments aka "journals"

Screenshots

Screenshot 2024-10-25 at 6 41 08 PM

What approach did you choose and why?

A default set of reactions is defined and rendered out as unicode points; storing the reaction strings and evaluating the corresponding unicode points allows for more flexibility for emoji modifiers. E.g. adding skin-tone support should also be evaluated at run time based on the users preference.

Merge checklist

  • Added/updated tests
  • Tested major browsers (Chrome, Firefox, Safari, ...)

@jjabari-op jjabari-op self-assigned this Jul 2, 2024
@jjabari-op jjabari-op marked this pull request as draft July 2, 2024 19:42
…e/40437-emoji-reactions-to-work-package-comments
@akabiru akabiru self-assigned this Sep 30, 2024
jjabari-op and others added 12 commits October 1, 2024 20:56
…e/40437-emoji-reactions-to-work-package-comments
…e/40437-emoji-reactions-to-work-package-comments
…e/40437-emoji-reactions-to-work-package-comments
…e/40437-emoji-reactions-to-work-package-comments
Users who do not have commenting permissions: typically the `add_work_package_notes` and anonymous
users cannot add new reactions but they can view any existing reactions. Futher, the "Add reactions" button
is also hidden from them.

In view only state, the emoji reactions render as primer invisible buttons in disabled state.

> They can respond to user input and provide feedback, but it does not trigger any actions.
> See: https://primer.style/components/button#inactive

The view only reactions mimic primer inactive buttons but without the muted styling and cursor change
@akabiru akabiru force-pushed the feature/40437-emoji-reactions-to-work-package-comments branch from 466a722 to b08ed5b Compare October 24, 2024 18:35
Emoji Reactions should always be returned in their current state, timeslicing will yield incorrect results.
At any given time, we ONLY need to know what reactions a Journal has. Further, EmojiReactions are never updated, rather created
and destroyed.

Retain timeslicing only for the journal render- where that is relevant to find updated journals
Journalsa are updated on last updated_at; reactions cannot rely on this as they do not "touch"
the journal, so a full journal refresh would not be viable. We avoid "touching" the journal on emoji
reactions update to retain concurrent user actions. E.g. If a user has a journal in edit mode, we do not want to overrite
it on the next polling update- this is why it's safer to stream emoji reactions separately

This reverts commit fb419dd.
@akabiru akabiru force-pushed the feature/40437-emoji-reactions-to-work-package-comments branch from bc0ed1a to a302f3f Compare October 25, 2024 12:00
@akabiru akabiru marked this pull request as ready for review October 25, 2024 13:01
@akabiru akabiru requested a review from Kharonus October 25, 2024 13:01
Copy link
Member

@Kharonus Kharonus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only a few found in pair review

app/models/emoji_reaction.rb Show resolved Hide resolved
config/locales/en.yml Outdated Show resolved Hide resolved
config/locales/en.yml Outdated Show resolved Hide resolved
spec/contracts/emoji_reactions/base_contract_spec.rb Outdated Show resolved Hide resolved
@akabiru akabiru force-pushed the feature/40437-emoji-reactions-to-work-package-comments branch from 0d7d897 to 403dbd7 Compare October 25, 2024 15:23
@akabiru akabiru force-pushed the feature/40437-emoji-reactions-to-work-package-comments branch from 403dbd7 to 88047f1 Compare October 25, 2024 15:27
Use both hex codes to ensure red color! Tested on Safari
@akabiru akabiru force-pushed the feature/40437-emoji-reactions-to-work-package-comments branch from 96e1421 to a6427d3 Compare October 25, 2024 15:49
@akabiru akabiru merged commit 87a8334 into dev Oct 25, 2024
12 checks passed
@akabiru akabiru deleted the feature/40437-emoji-reactions-to-work-package-comments branch October 25, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants