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.

Sign up for GitHub

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

Jump to bottom

feat: Reconnect logic #9453

Merged
merged 57 commits into from
Jun 3, 2024
Merged

feat: Reconnect logic #9453

merged 57 commits into from
Jun 3, 2024

Conversation

iamsivin
Copy link
Member

@iamsivin iamsivin commented May 10, 2024 •
edited
Loading

Pull Request Template

Description

Fixes https://linear.app/chatwoot/issue/CW-1957/web-app-disconnects-and-doesnt-notify-about-incoming-messages-anymore

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Loom Video
https://www.loom.com/share/076018aa84e248ae806fe3bd865165a1?sid=4e25a798-8796-4b1f-8f02-3c0013862d2f

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@linear Linear
Copy link

linear bot commented May 10, 2024

CW-1957 Web app disconnects and doesn't notify about incoming messages anymore

@iamsivin iamsivin requested a review from pranavrajs May 10, 2024 13:47
@iamsivin iamsivin temporarily deployed to chatwoot-pr-9453 May 10, 2024 13:54 Inactive
@iamsivin iamsivin temporarily deployed to chatwoot-pr-9453 May 10, 2024 16:34 Inactive
@iamsivin iamsivin temporarily deployed to chatwoot-pr-9453 May 10, 2024 17:03 Inactive
@iamsivin iamsivin marked this pull request as ready for review May 14, 2024 07:53
@iamsivin iamsivin mentioned this pull request May 14, 2024
Closed
7 tasks
@iamsivin iamsivin temporarily deployed to staging-chatwoot May 14, 2024 14:14 Inactive
@iamsivin iamsivin changed the title feat: Updates reconnect logic feat: Reconnect logic May 14, 2024
@muhsin-k muhsin-k self-requested a review May 30, 2024 10:32
muhsin-k
muhsin-k requested changes May 30, 2024
View reviewed changes
Copy link
Member

@muhsin-k muhsin-k left a comment

Choose a reason for hiding this comment

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

@iamsivin Please see the inline comments.

@iamsivin iamsivin requested a review from muhsin-k May 30, 2024 12:20
@muhsin-k
Copy link
Member

@iamsivin Message are updating after the disconnect. But the loader is not closing for some reason.

chat-message-loading.mov

@iamsivin iamsivin requested a review from muhsin-k June 3, 2024 07:12
@iamsivin
Copy link
Member Author

iamsivin commented Jun 3, 2024

@iamsivin Message are updating after the disconnect. But the loader is not closing for some reason.

chat-message-loading.mov

@muhsin-k Nice catch! Updated the PR with a fix.

scmmishra
scmmishra reviewed Jun 3, 2024
View reviewed changes
Copy link
Member

@scmmishra scmmishra left a comment

Choose a reason for hiding this comment

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

@iamsivin overall the code looks good to go, just had one suggestion, feel free to take that up separately

@scmmishra
feat: use composables that automatically cleanup listeners on the `on…
534416d 
…BeforeUnmount` hook (#9581)

This PR makes the following changes

1. Add `@vueuse/core` package, it supports Vue 2.7, we should start
using this across the board
2. Add `useEmitter` composable, this adds a listener on the emitter on
`onMounted` and automatically removes it on `onBeforeUnmount`
3. Use `useEmitter` & `useEventListener` from in
`NetworkNotification.vue`
@iamsivin iamsivin requested a review from scmmishra June 3, 2024 08:32
@muhsin-k muhsin-k merged commit af90f21 into develop Jun 3, 2024
14 of 15 checks passed
@muhsin-k muhsin-k deleted the feat/CW-1957 branch June 3, 2024 10:24
@pranavrajs pranavrajs mentioned this pull request Jun 11, 2024
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@muhsin-k muhsin-k

@scmmishra scmmishra

@pranavrajs pranavrajs

@sojan-official sojan-official

Assignees

@scmmishra scmmishra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iamsivin @muhsin-k @scmmishra @vishnu-narayanan