Add a communication channel for forcing reloads across tab sessions.
Review Request #13048 — Created May 16, 2023 and updated
Information | |
---|---|
david | |
Review Board | |
release-6.x | |
Reviewers | |
reviewboard | |
The new behavior for the unified banner means that individual tabs fetch
and keep track of pending review/draft IDs more eagerly than before. The
result of this is that it was easier to hit bugs when performing actions
with a review request open in multiple tabs.This change adds a small utility that makes use of the
BroadcastChannel
API to coordinate across multiple tabs/windows within
a browser session. Right now the only message that we send is "reload",
which asks other tabs to reload their contents. This is sent when
something is either published or discarded, which invalidates draft IDs.The broadcast channel is origin-wide, so each
PageView
can implement a
method to return a data blob that's used for filtering whether or not to
reload. The default implementation is empty, andReviewablePageView
will filter based on the review request ID (so any open tabs related to
that review request will be triggered).
- Ran js-tests.
- Published review requests, reviews, and replies from one tab, and saw
other tabs pop up the alert and reload. - Discarded drafts from the unified banner and from the review dialog,
and saw other tabs pop up the alert and reload.
Summary | |
---|---|
Description | From | Last Updated |
---|---|---|
We're going to need tests for this, which will be especially important for any filtering logic and any future use … |
|
|
I also want to discuss naming. Eventually, we'll want an open comm channel to the Review Board server as well. … |
|
|
Can you add a Version Added? |
|
|
This is missing docs. Should also have Version Added, which I've unfortunately left out of my other TS reviews. |
|
|
Missing Version Added. |
|
|
Should document this. |
![]() |
|
Should document this. |
![]() |
|
I think you could get rid of this and replace it with some info about how we're going to trigger … |
![]() |
-
-
We're going to need tests for this, which will be especially important for any filtering logic and any future use of the broadcast channel.
-
I also want to discuss naming. Eventually, we'll want an open comm channel to the Review Board server as well. Is this going to be designed with the intent of being used for that as well? Or should we name this something more specific?
-
reviewboard/static/rb/js/common/models/commChannel.ts (Diff revision 1) Can you add a
Version Added
? -
reviewboard/static/rb/js/common/models/commChannel.ts (Diff revision 1) This is missing docs. Should also have
Version Added
, which I've unfortunately left out of my other TS reviews. -
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+494) |
Checks run (2 succeeded)

-
-
-
-
reviewboard/static/rb/js/reviews/views/reviewablePageView.ts (Diff revision 2) I think you could get rid of this and replace it with some info about how we're going to trigger tabs related to the review request.
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+498) |