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 composer link preview overridden by previous enrichment #3025

Merged
merged 6 commits into from Feb 12, 2024

Conversation

nuno-vieira
Copy link
Member

@nuno-vieira nuno-vieira commented Feb 12, 2024

🔗 Issue Links

None

🎯 Goal

Fix composer link preview overriding actual current link preview

📝 Summary

  • Increases the debouncer to 0.4s to be more aligned with how usually requests take to complete
  • Fix composer link preview overriding actual current link preview

🛠 Implementation

The proper way to do this is to cancel the previous request whenever we send a new request to enrich the url. The problem is that right now we do not have support for cancellable requests in our SDK, mostly because of tech debt issues. So right now, the best way to improve this is when we show the link preview we need to make sure that url is still the one in the composer input.

🧪 Manual Testing Notes

Link preview should not show after sending the message

  1. Type a valid link in the composer
  2. Very quickly after typing it, send the message
  3. After the message was sent, it should not show the link preview

Valid old link preview should not override current link preview

  1. Type a link in the composer
  2. Very quickly type a new one
  3. Only the new one should be shown

Incorrect old link preview should not override current link preview

  1. Type an invalid link in the composer
  2. Very quickly type a new valid one
  3. Only the new one should be shown (The preview should not be dismissed)

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change follows zero ⚠️ policy (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@nuno-vieira nuno-vieira added 🐞 Bug An issue or PR related to a bug 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK labels Feb 12, 2024
@nuno-vieira nuno-vieira requested a review from a team as a code owner February 12, 2024 15:46
Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

LGTM! ✅ Just please update the changelog entry a bit.

CHANGELOG.md Outdated Show resolved Hide resolved
@nuno-vieira nuno-vieira changed the title Fix composer link preview overriding actual current link preview Fix composer link preview overridden by previous enrichment Feb 12, 2024
@nuno-vieira nuno-vieira added the 🤞 Ready For QA A PR that is Ready for QA label Feb 12, 2024
Copy link

sonarcloud bot commented Feb 12, 2024

@testableapple testableapple added 🟢 QAed A PR that was QAed and removed 🤞 Ready For QA A PR that is Ready for QA labels Feb 12, 2024
Copy link
Contributor

@testableapple testableapple left a comment

Choose a reason for hiding this comment

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

🚢

@nuno-vieira nuno-vieira merged commit 41a6917 into develop Feb 12, 2024
13 of 15 checks passed
@nuno-vieira nuno-vieira deleted the fix/old-composer-link-preview-overrides-newest branch February 12, 2024 17:49
@nuno-vieira nuno-vieira mentioned this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug 🟢 QAed A PR that was QAed 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants