-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Refactor RoomStatusBar into MVVM #31523
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
base: develop
Are you sure you want to change the base?
Conversation
| import { MatrixClientPeg } from "../../src/MatrixClientPeg"; | ||
| import { mkThread } from "../test-utils/threads"; | ||
|
|
||
| describe("getUnsentMessages", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a copy paste of the tests to match the move of getUnsentMessages to the other file.
| describe("computed attributes", () => { | ||
| beforeEach(() => { | ||
| jest.spyOn(RoomStatusBarModule, "getUnsentMessages").mockReturnValue([]); | ||
| jest.spyOn(room, "getPendingEvents").mockReturnValue([]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spying on the whole module was unnessacery when the influencing factor of getUnsentMessages was mostly whether getPendingEvents had any events.
It also broke due to the fragility mocking whole imports, so I've converted it to a much more stable mock.
amshakal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Refactoring the room status bar into a proper shared-component / MVVM thing work will be used to eventually start rendering safety errors for MSC4387.
This also changes the styling of quite a few of our room status banners into proper Compound ones, utilising the Banner.
See snapshots for changes