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

Bug: Account syncing notification behaviour #1367

Closed
justinfar opened this issue Oct 25, 2024 · 3 comments
Closed

Bug: Account syncing notification behaviour #1367

justinfar opened this issue Oct 25, 2024 · 3 comments

Comments

@justinfar
Copy link

Describe the bug
The behaviour of the notification when trying to sync accounts is a bit erratic as it resizes in width a few times, and just shows multiple Sync complete notifications until timing out before the success message can be even read.

A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Settings/Accounts
  2. Click on Sync All
  3. See notification stack and quick timeout

Expected behavior

  1. Click Sync
  2. A single notfication is shown where the state changes from Syncing account balances to Syncing complete/failed (depending on state)

What version of Maybe are you using?
Hosted" (i.e. app.maybe.co)

Screenshots / Recordings
https://github.com/user-attachments/assets/2c653863-9d25-4dfc-a0e6-bd4e80020638

@zachgoll
Copy link
Collaborator

For anyone looking into this, here are the relevant lines that trigger the Turbo Stream updates during the sync:

def broadcast_start
broadcast_append_to(
[ account.family, :notifications ],
target: "notification-tray",
partial: "shared/notification",
locals: { id: id, type: "processing", message: "Syncing account balances" }
)
end
def broadcast_result(type:, message:)
broadcast_remove_to account.family, :notifications, target: id # Remove persistent syncing notification
broadcast_append_to(
[ account.family, :notifications ],
target: "notification-tray",
partial: "shared/notification",
locals: { type: type, message: message }
)
account.family.broadcast_refresh
end

@Awlter
Copy link

Awlter commented Nov 12, 2024

I have checked the code, it seems it's more of a design issue. What's the ultimate expected behavior after clicking the sync all button?

@zachgoll
Copy link
Collaborator

#1433 introduces a new way of syncing and should eliminate this issue, so I'm going to close this out to avoid merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants