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: Set conditional wrapping for big messages on PDF transcript's react template #32311

Merged
merged 19 commits into from
May 9, 2024

Conversation

KevLehman
Copy link
Contributor

@KevLehman KevLehman commented Apr 24, 2024

https://rocketchat.atlassian.net/browse/CORE-75

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Main problem was caused by a set of (the react-pdf equivalent of a div> for our purposes) elements that were not allowed to break down (wrap=false) since they were holding together the structure of a message. This was defined like this so when PDF was near end of page, we would move the whole "message" box to the next page.
However, when a message (in this case, a quoted one) was bigger than the available size in the page, the library knew it had to break, but since the whole view was unbreakable, it tried to move to the next page infinitely.

Template was updated to have a better shape and prevent this from happening, as well as unit tests with some other edge cases related to this specific issue.

Copy link

changeset-bot bot commented Apr 24, 2024

🦋 Changeset detected

Latest commit: e0fcda7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-services Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/apps Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/models Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Apr 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.60%. Comparing base (3b3275f) to head (e0fcda7).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #32311      +/-   ##
===========================================
+ Coverage    55.57%   55.60%   +0.02%     
===========================================
  Files         2401     2405       +4     
  Lines        52858    52929      +71     
  Branches     10857    10872      +15     
===========================================
+ Hits         29375    29430      +55     
- Misses       20891    20899       +8     
- Partials      2592     2600       +8     
Flag Coverage Δ
e2e 54.93% <ø> (+0.05%) ⬆️
unit 73.40% <ø> (+<0.01%) ⬆️

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

Copy link
Contributor

dionisio-bot bot commented Apr 25, 2024

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@KevLehman KevLehman changed the title fix: pdf fix: Multiple issues causing PDF generation to die in the middle of processing Apr 26, 2024
@KevLehman KevLehman changed the title fix: Multiple issues causing PDF generation to die in the middle of processing fix: Set conditional wrapping for big messages on PDF transcript's react template Apr 26, 2024
@KevLehman KevLehman added this to the 6.9 milestone Apr 26, 2024
@KevLehman KevLehman marked this pull request as ready for review April 26, 2024 19:41
@jessicaschelly jessicaschelly added the stat: QA assured Means it has been tested and approved by a company insider label May 9, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label May 9, 2024
@kodiakhq kodiakhq bot merged commit ad86761 into develop May 9, 2024
43 checks passed
@kodiakhq kodiakhq bot deleted the fix/pdf-again branch May 9, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants