Fixed UI Inconsistencies while quoting messages #605

devanshkansagra · 2024-07-09T08:16:19Z

Acceptance Criteria fulfillment

Added support for quoting different types of attachements
Implemented displaying of attachement while quoting
Fix recursive quoting
Fix design of quoted message in curved variants

Fixes #563

Video/Screenshots

Screencast.from.2024-07-14.16-41-33.webm

Spiral-Memory · 2024-07-09T08:22:10Z

Hey @devanshkansagra,

Thanks for the contribution.

While quoting messages, there's a sideline that appears and quotes the message. It seems like you simply removed it, which shouldn't be the case. Please also try it in Storybook so that we can see the changes across all design variants. It must work properly in all variants. Additionally, I don't think it's a good idea to show thumbnails in "QuoteBox." Images look fine, but video and audio appear way too big.

devanshkansagra · 2024-07-09T08:25:34Z

Okay will check that

devanshkansagra · 2024-07-10T09:59:26Z

devanshkansagra · 2024-07-11T16:26:47Z

Hey maintainers, I have fixed the issue of recursive quoting, but I got stucked at styling of recursive quoting of messages in curved variants, I have explored a lot, but I didn't get any clue. So anyone could please suggest me, what kind of design improvements I need to make to fix this

This is the design issue in curved variant

It looks proper in other variants (Ex StormySeas)

Spiral-Memory · 2024-07-11T16:30:02Z

Hey @devanshkansagra It looks good !

Let me know the exact problem you are facing in fixing the design in curved variant as well

Spiral-Memory · 2024-07-11T16:30:28Z

Also please update the video with your latest changes

devanshkansagra · 2024-07-11T16:31:15Z

Hey @devanshkansagra It looks good !

Let me know the exact problem you are facing in fixing the design in curved variant as well

Most basically the improper alignment in curved variants

devanshkansagra · 2024-07-11T16:48:16Z

Also please update the video with your latest changes

Updated the video

Spiral-Memory · 2024-07-11T16:53:03Z

Also please update the video with your latest changes

Updated the video

Checked your video, Great work!

Can we fix this as well before fixing for the curved variant, that it should display the actual message here instead of msg link:

Also, in recursive quoting, the video / image is not visible, I know you have let me know about it that even with main RC it doesn't work, but it would be great if you find a fix for that as well if possible.

Thanks!

devanshkansagra · 2024-07-11T17:36:57Z

Also please update the video with your latest changes

Updated the video

Checked your video, Great work!

Can we fix this as well before fixing for the curved variant, that it should display the actual message here instead of msg link:

Also, in recursive quoting, the video / image is not visible, I know you have let me know about it that even with main RC it doesn't work, but it would be great if you find a fix for that as well if possible.

Thanks!

will fix and make the recursive quote preview look like RC

devanshkansagra · 2024-07-12T20:37:59Z

devanshkansagra · 2024-07-14T11:26:22Z

Hey maintainers, Can you review my pull request? So that I can follow up any changes if needed

Spiral-Memory · 2024-07-14T11:32:03Z

Hey @devanshkansagra

I watched your video, it's looking good ! However in recursive quoting, attachment is not getting displayed right ?

I'll review the code to see if it's not breaking anything..

Thanks a lot for the contribution !

devanshkansagra · 2024-07-14T11:33:13Z

Hey @devanshkansagra

I watched your video, it's looking good ! However in recursive quoting, attachment is not getting displayed right ?

I'll review the code to see if it's not breaking anything..

Thanks a lot for the contribution !

This issue is there with the main RC, hence it is not able to display the attachment while recursive quoting

Spiral-Memory · 2024-07-14T11:43:00Z

Hey @devanshkansagra

I watched your video, it's looking good ! However in recursive quoting, attachment is not getting displayed right ?

I'll review the code to see if it's not breaking anything..

Thanks a lot for the contribution !

This issue is there with the main RC, hence it is not able to display the attachment while recursive quoting

Ohh Okay !

Let me see it, I'll let you know

devanshkansagra · 2024-07-14T11:43:33Z

Sure

Spiral-Memory · 2024-07-15T17:02:29Z

packages/react/src/views/AttachmentHandler/Attachment.js

@@ -14,6 +14,8 @@ const Attachment = ({ attachment, host, type, variantStyles = {} }) => {
        attachment={attachment}
        host={host}
        variantStyles={variantStyles}
+        authorIcon={attachment.author_icon}


Send only one prop, i.e., author or authorInfo, and destructure where required. If possible, try not to send extra props. If required, send author.

Okay will change this

Spiral-Memory · 2024-07-15T17:04:20Z

packages/react/src/views/AttachmentHandler/Attachment.js

@@ -44,6 +51,42 @@ const Attachment = ({ attachment, host, type, variantStyles = {} }) => {
      />
    );
  }
+  if (attachment && attachment.attachments[0]?.image_url) {


Why introduce redundant rendering of a component? Instead, you can add the required condition where we are already using ImageAttachment, AudioAttachment, and VideoAttachment.

It was easier for me to implement like this and this is also a conditional based rendering

Spiral-Memory · 2024-07-15T17:05:29Z

packages/react/src/views/AttachmentHandler/AudioAttachment.js

+}) => {
+  const { RCInstance } = useContext(RCContext);
+  const { colors } = useTheme();
+  const getUserAvatarUrl = (icon) => {


prop name such as authorUrl here will be better.

Okay will change this

packages/react/src/views/AttachmentHandler/TextAttachment.js

Spiral-Memory · 2024-07-15T17:07:52Z

packages/react/src/views/QuoteMessage/QuoteMessage.js

    return URL;
  };
  const styles = useQuoteMessageStyles();
  const setQuoteMessage = useMessageStore((state) => state.setQuoteMessage);

  const { classNames, styleOverrides } = useComponentOverrides('QuoteMessage');
+
+  // Function to render the message content or attachment preview
+  const renderMessageContent = () => {


Let's not have a function here like this, try using JSX with required conditions

Okay will modify this

… the message is quoted, also implemented the preview of image, audio and video attachment while quoting

…display while quoting

…riants

…Icon

Spiral-Memory · 2024-10-01T08:05:09Z

Thank you for contribution @devanshkansagra
Merging it for testing, will revert back if there are issues

This reverts commit d84157f.

devanshkansagra marked this pull request as ready for review July 13, 2024 12:37

devanshkansagra changed the title ~~Fixes UI Inconsistencies while quoting messages~~ Fixed UI Inconsistencies while quoting messages Jul 13, 2024

devanshkansagra marked this pull request as draft July 13, 2024 19:06

devanshkansagra marked this pull request as ready for review July 14, 2024 11:25

Spiral-Memory reviewed Jul 15, 2024

View reviewed changes

devanshkansagra added 9 commits September 30, 2024 14:38

Fixed the issue of not displaying image, video and audio preview when…

1cf7fc2

… the message is quoted, also implemented the preview of image, audio and video attachment while quoting

Formatted using prettier

43079bc

Fixed lint error

456f7eb

Added border in quoted attachments to look proper

5884464

Fixed Lint error

34bf50f

Fixed no-shadow and no-else-return lint issue

c719df9

Implemented Recursive quoting

613239d

Implemented recursive quoted message preview and fixed unwanted link …

fe8c398

…display while quoting

CSS fix for recursive quoting in curved variants

22362ef

devanshkansagra added 5 commits September 30, 2024 14:40

Fixed the curved appearence in recursive quoted messages in curved va…

586850e

…riants

Fixed the rendering of quoted image while quoting

a2ed7d0

Minor Fix

9fc4908

Removed renderMessageContent and used JSX

9134057

Using one prop author in attachments instead of authorName and author…

3c7c200

…Icon

devanshkansagra force-pushed the fix#563 branch from 343eb4f to 3c7c200 Compare September 30, 2024 09:12

devanshkansagra and others added 2 commits September 30, 2024 14:51

Fixed after conflicts issue

5b8e7d4

Merge branch 'develop' into fix#563

af4edbe

Spiral-Memory approved these changes Oct 1, 2024

View reviewed changes

Spiral-Memory merged commit d84157f into RocketChat:develop Oct 1, 2024
3 checks passed

Spiral-Memory added a commit that referenced this pull request Oct 1, 2024

Revert "Fixed UI Inconsistencies while quoting messages (#605)"

cfc6c3e

This reverts commit d84157f.

Spiral-Memory mentioned this pull request Oct 1, 2024

Revert "Fixed UI Inconsistencies while quoting messages" #632

Merged

Spiral-Memory added a commit that referenced this pull request Oct 1, 2024

Revert "Fixed UI Inconsistencies while quoting messages (#605)" (#632)

df09c9d

This reverts commit d84157f.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed UI Inconsistencies while quoting messages #605

Fixed UI Inconsistencies while quoting messages #605

devanshkansagra commented Jul 9, 2024 •

edited

Loading

Spiral-Memory commented Jul 9, 2024

devanshkansagra commented Jul 9, 2024

devanshkansagra commented Jul 10, 2024

devanshkansagra commented Jul 11, 2024 •

edited

Loading

Spiral-Memory commented Jul 11, 2024

Spiral-Memory commented Jul 11, 2024

devanshkansagra commented Jul 11, 2024 •

edited

Loading

devanshkansagra commented Jul 11, 2024

Spiral-Memory commented Jul 11, 2024

devanshkansagra commented Jul 11, 2024

devanshkansagra commented Jul 12, 2024 •

edited

Loading

devanshkansagra commented Jul 14, 2024 •

edited

Loading

Spiral-Memory commented Jul 14, 2024

devanshkansagra commented Jul 14, 2024 •

edited

Loading

Spiral-Memory commented Jul 14, 2024

devanshkansagra commented Jul 14, 2024

Spiral-Memory Jul 15, 2024

devanshkansagra Jul 15, 2024

Spiral-Memory Jul 15, 2024

devanshkansagra Jul 15, 2024

Spiral-Memory Jul 15, 2024

devanshkansagra Jul 15, 2024

Spiral-Memory Jul 15, 2024

devanshkansagra Jul 15, 2024

Spiral-Memory commented Oct 1, 2024 •

edited

Loading

Fixed UI Inconsistencies while quoting messages #605

Fixed UI Inconsistencies while quoting messages #605

Conversation

devanshkansagra commented Jul 9, 2024 • edited Loading

Acceptance Criteria fulfillment

Video/Screenshots

Spiral-Memory commented Jul 9, 2024

devanshkansagra commented Jul 9, 2024

devanshkansagra commented Jul 10, 2024

devanshkansagra commented Jul 11, 2024 • edited Loading

Spiral-Memory commented Jul 11, 2024

Spiral-Memory commented Jul 11, 2024

devanshkansagra commented Jul 11, 2024 • edited Loading

devanshkansagra commented Jul 11, 2024

Spiral-Memory commented Jul 11, 2024

devanshkansagra commented Jul 11, 2024

devanshkansagra commented Jul 12, 2024 • edited Loading

devanshkansagra commented Jul 14, 2024 • edited Loading

Spiral-Memory commented Jul 14, 2024

devanshkansagra commented Jul 14, 2024 • edited Loading

Spiral-Memory commented Jul 14, 2024

devanshkansagra commented Jul 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spiral-Memory commented Oct 1, 2024 • edited Loading

devanshkansagra commented Jul 9, 2024 •

edited

Loading

devanshkansagra commented Jul 11, 2024 •

edited

Loading

devanshkansagra commented Jul 11, 2024 •

edited

Loading

devanshkansagra commented Jul 12, 2024 •

edited

Loading

devanshkansagra commented Jul 14, 2024 •

edited

Loading

devanshkansagra commented Jul 14, 2024 •

edited

Loading

Spiral-Memory commented Oct 1, 2024 •

edited

Loading