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

[NEW] Attachment Files Redesign #27470

Merged
merged 30 commits into from
Jan 18, 2023
Merged

[NEW] Attachment Files Redesign #27470

merged 30 commits into from
Jan 18, 2023

Conversation

hugocostadev
Copy link
Contributor

@hugocostadev hugocostadev commented Dec 6, 2022

Proposed changes (including videos or screenshots)

  • Replace File attachments to use MessageGenericPreview
  • Move File attachments to the /MessageList folder
  • Refactor Video, Image, and Audio Attachments to use MessageGenericPreview
  • New component MessageCollapsible
  • Refactored the OEmbedCollpsible component to use MessageCollapsible
  • Refactored the AttachmentSize component to include different style
  • Added descriptionMd property to the MessageAttachment object to use the new parser in descriptions
  • Added size and format to attachment object at sendFileMessage.ts
  • Changed maxWidth and maxHeight of AttachmentContext to be the same as the OEmbed sizes

OLD:
image

NEW:
image

Issue(s)

Steps to test or reproduce

Further comments

This PR implements the File Attachment using the MessageGenericPreview.
To be able to test during the Draft/PR phase, it's necessary to run both PR together.

Fuselage Pull Request: RocketChat/fuselage#932

TC-57

@lgtm-com
Copy link

lgtm-com bot commented Dec 6, 2022

This pull request introduces 1 alert when merging 64d41eb into c8af645 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

hugocostadev and others added 4 commits December 6, 2022 15:23
…/attachment-file

* 'develop' of github.com:RocketChat/Rocket.Chat: (33 commits)
  Chore: fix EmailInbox intermittent e2e tests (#27573)
  [IMPROVE] Authorize search of custom fields on `users.list` (#27423)
  Chore:  Threads as React components (#27524)
  [NEW] Upload service (#27543)
  [FIX] Fix emoji appearance on sidebar (#27580)
  [FIX] RoomLeader status not working (#27576)
  [FIX] Announcement banner link opening in the same page (#27554)
  Bump version to 5.4.1
  [FIX] Custom languages not being applied to i18next (#27557)
  [FIX] Registration and Login placeholders not being used (#27558)
  [FIX] Fix Login with Show default form disabled (#27475)
  [FIX] Message Actions menu does not close upon choosing an action (#27328)
  Chore: Deprecate unused omnichannel API (#27538)
  [FIX] Pagination not working on current chats (#27432)
  [FIX] Custom languages not being applied to i18next (#27557)
  [FIX] Registration and Login placeholders not being used (#27558)
  [FIX] Message Actions menu does not close upon choosing an action (#27328)
  Chore: Deprecate unused omnichannel API (#27538)
  Regression: Add button-icon-disabled-color to the palette (#27522)
  Chore: Refactor CreateChannelModal (#27469)
  ...
@gabriellsh gabriellsh marked this pull request as ready for review December 20, 2022 20:31
@gabriellsh gabriellsh requested review from a team as code owners December 20, 2022 20:31
@gabriellsh
Copy link
Member

We should discuss better if descriptionMd is something we want to do...

@codecov
Copy link

codecov bot commented Dec 29, 2022

Codecov Report

Merging #27470 (b56ac3e) into develop (1fecf0f) will increase coverage by 0.90%.
The diff coverage is 22.22%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27470      +/-   ##
===========================================
+ Coverage    42.01%   42.92%   +0.90%     
===========================================
  Files          844      818      -26     
  Lines        17751    17243     -508     
  Branches      2009     1939      -70     
===========================================
- Hits          7458     7401      -57     
+ Misses       10029     9580     -449     
+ Partials       264      262       -2     
Flag Coverage Δ
e2e 42.92% <22.22%> (+0.90%) ⬆️

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

Copy link
Member

@gabriellsh gabriellsh left a comment

Choose a reason for hiding this comment

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

When sending a giph using the giphy app, the aspect ratio seems to be lost.

image

Develop:
image

Your PR:
image

Other attachments seem to be fine though.

@hugocostadev hugocostadev reopened this Jan 5, 2023
@hugocostadev hugocostadev added this to the 6.0.0 milestone Jan 5, 2023
gabriellsh
gabriellsh previously approved these changes Jan 5, 2023
tassoevan
tassoevan previously approved these changes Jan 9, 2023
tassoevan
tassoevan previously approved these changes Jan 13, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: conflict labels Jan 17, 2023
@kodiakhq kodiakhq bot merged commit 9fa9661 into develop Jan 18, 2023
@kodiakhq kodiakhq bot deleted the feat/attachment-file branch January 18, 2023 13:59
gabriellsh added a commit that referenced this pull request Jan 23, 2023
…etChat/Rocket.Chat into matrixSearch

* 'feat/federation-search-public-rooms' of github.com:RocketChat/Rocket.Chat:
  Chore: Move service shutdown logic to each service (#27690)
  Chore: change colors to dark theme on Marketplace (#27532)
  Chore: Update color tokens (#27704)
  Chore: Remove medium prop from ButtonGroup  (#27784)
  Regression: Add support for 2FA errors to `Meteor.callAsync` (#27767)
  [NEW] Attachment Files Redesign (#27470)
  [FIX] App page showing version undefined for apps not in marketplace (#27766)
  Chore: Add surface-light background to AppRow (#27765)
@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
13 tasks
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.

6 participants