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

🌟 Added remove link preview #2238

Merged
merged 2 commits into from
Jun 9, 2022
Merged

Conversation

rezk2ll
Copy link
Member

@rezk2ll rezk2ll commented Jun 7, 2022

Description

  • adds the ability to remove a link preview from the generated previews for a given message

Related Issue

#2235

Motivation and Context

  • allows users to control what previews to keep

How Has This Been Tested?

  • locally

Screenshots (if appropriate):

removeLinkPreview.mp4

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added the Signed-off-by statement at the end of my commit message.

@rezk2ll rezk2ll linked an issue Jun 7, 2022 that may be closed by this pull request
7 tasks
@rezk2ll rezk2ll requested review from RomaricMourgues and romanesko and removed request for RomaricMourgues June 7, 2022 17:40
@RomaricMourgues
Copy link
Contributor

Perfect, I'll review the code itself tomorrow

@RomaricMourgues
Copy link
Contributor

RomaricMourgues commented Jun 7, 2022

@shepilov so this was the last part of this feature according to me, could be nice to get the design from @BastiaanVanGaalen here too to finalise for real

@RomaricMourgues
Copy link
Contributor

@rezk2ll I forgot to ask, on the video it seems a bit slow to generate the links, do you have any idea what is taking much of this time ? Is it rabbitmq stuff or downloading the content of the link ?

@rezk2ll
Copy link
Member Author

rezk2ll commented Jun 8, 2022

there is no downloading happening, just HTML parsing and 1 call to get the favicon

I recorded another video it took about ~4 seconds for the preview to be displayed. from the console, I think most of the time is spent on parsing the HTML of the URL.

preview-test-speed.mp4

Copy link
Contributor

@RomaricMourgues RomaricMourgues left a comment

Choose a reason for hiding this comment

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

💯

@RomaricMourgues RomaricMourgues merged commit 35a17dc into linagora:develop Jun 9, 2022
@RomaricMourgues RomaricMourgues added the qa:ready When a PR is ready to go to QA label Jun 9, 2022
RomaricMourgues added a commit that referenced this pull request Jun 9, 2022
Co-authored-by: Romaric Mourgues <rmourgues@linagora.com>
@github-actions github-actions bot added staging:qa and removed qa:ready When a PR is ready to go to QA staging:develop labels Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LinksPreview::Ability to remove the attached link
3 participants