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

fix: Thread message preview #28454

Merged
merged 18 commits into from
Mar 22, 2023
Merged

fix: Thread message preview #28454

merged 18 commits into from
Mar 22, 2023

Conversation

hugocostadev
Copy link
Contributor

@hugocostadev hugocostadev commented Mar 16, 2023

Proposed changes (including videos or screenshots)

The thread preview message was not displaying emojis and when the parent message was a quoted message it was not displaying the correct message.

To fix the issues I did:

  • Emoji Issue: I used the <GazzodownText /> component to call <PreviewMarkup /> conditionally when passed the prop preview. The <GazzodownText /> component it's necessary because it has the MarkupInteractionContext.Provider that has the context necessary to display the emoji and other elements
  • Empty parent message: Since the parent message it's a quote, the initial string from a quote message it's a link that references the original message followed by \n break line. I added a validation to check if it's a quoteAttachment message, if it is I remove the first markdown token responsible by the link

Before:
image

After:
image

Issue(s)

Steps to test or reproduce

  1. Open https://open.rocket.chat
  2. Send a message
  3. Quote the previous message
  4. Send another sequential message
  5. Reply in thread the message sent on step 3
  6. Write the reply message inside the thread and select the checkbox Also send to channel
  7. Check it how it looks the Thread Message Preview in the Message List

Further comments

Little improvements made:

  • Memoized <ThreadMessagePreview /> and <ThreadMessagePreviewBody /> components to avoid component re-renders

NOTE: The extra spaces saw in the Before image it's being fixed in a different PR (#28434)

TC-550

@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #28454 (2a0f1f1) into develop (17ac357) will increase coverage by 10.66%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop   #28454       +/-   ##
============================================
+ Coverage    34.31%   44.98%   +10.66%     
============================================
  Files          673      755       +82     
  Lines        13547    14622     +1075     
  Branches      2036     2108       +72     
============================================
+ Hits          4649     6577     +1928     
+ Misses        8644     7747      -897     
- Partials       254      298       +44     
Flag Coverage Δ
e2e 44.93% <ø> (+10.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Mar 20, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 20, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 20, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 20, 2023
@dionisio-bot dionisio-bot bot removed the stat: ready to merge PR tested and approved waiting for merge label Mar 20, 2023
@hugocostadev hugocostadev added the stat: ready to merge PR tested and approved waiting for merge label Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Mar 22, 2023
@kodiakhq kodiakhq bot merged commit 7675c1d into develop Mar 22, 2023
@kodiakhq kodiakhq bot deleted the fix/thread_preview branch March 22, 2023 20:01
gabriellsh added a commit that referenced this pull request Mar 23, 2023
…ove/full-name-username

* 'develop' of github.com:RocketChat/Rocket.Chat: (54 commits)
  chore: ignore `.eslintcache` (#28475)
  refactor: convert omnichannel callbacks to ts (#28564)
  refactor: Allow RoomCoordinator.roomFind to be async (#28566)
  refactor(models): Use Messages Raw model (4/N) (#28558)
  refactor(client): `meteor/session` usage (1/N) (#28565)
  refactor: Don't use meteor collections on migrations (#28563)
  feat: VideoConference Guest mode and Conference Router (#28186)
  refactor: promise.await removal 8/N (#28560)
  refactor: checkUsernameAvailability to async (#28557)
  refactor(promise.await): Low hanging fruits (2/2) (#28556)
  fix: Changing the app's error verification (#28450)
  refactor(promise.await): Low hanging fruits (1/N) (#28555)
  fix: Thread message preview (#28454)
  refactor: remove Promise.await from LDAP files (#28527)
  refactor: canAccessRoomId to async (#28540)
  refactor: Remove `LivechatInquiry` model (#28487)
  refactor(models): Use Messages Raw model (3/N) (#28549)
  refactor: remove Promise.await (#28539)
  chore: Make CI fail if checks fail (#28552)
  refactor: remove Promise.await from oauth manager (#28530)
  ...
gabriellsh added a commit that referenced this pull request Mar 23, 2023
…ixSearch

* 'develop' of github.com:RocketChat/Rocket.Chat: (141 commits)
  ci(docker): use correct var name
  fix: Prevent blank space on live chat form validations (#28243)
  ci(docker): Push Docker tag for commit hash (#28578)
  fix: marketplace doc link wrong redirect (#28466)
  refactor: move `Subscriptions` 1x (#28531)
  chore: ignore `.eslintcache` (#28475)
  refactor: convert omnichannel callbacks to ts (#28564)
  refactor: Allow RoomCoordinator.roomFind to be async (#28566)
  refactor(models): Use Messages Raw model (4/N) (#28558)
  refactor(client): `meteor/session` usage (1/N) (#28565)
  refactor: Don't use meteor collections on migrations (#28563)
  feat: VideoConference Guest mode and Conference Router (#28186)
  refactor: promise.await removal 8/N (#28560)
  refactor: checkUsernameAvailability to async (#28557)
  refactor(promise.await): Low hanging fruits (2/2) (#28556)
  fix: Changing the app's error verification (#28450)
  refactor(promise.await): Low hanging fruits (1/N) (#28555)
  fix: Thread message preview (#28454)
  refactor: remove Promise.await from LDAP files (#28527)
  refactor: canAccessRoomId to async (#28540)
  ...
@sampaiodiego sampaiodiego mentioned this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants