Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Conversation

@andybalaam
Copy link
Member

@andybalaam andybalaam commented Nov 25, 2021


This change is marked as an internal change (Task), so will not be included in the changelog.

Preview: https://61a0a702f2428631a8a14fd3--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@andybalaam andybalaam added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Nov 25, 2021
@andybalaam andybalaam requested a review from a team as a code owner November 25, 2021 15:57
@t3chguy t3chguy requested review from t3chguy and removed request for a team November 25, 2021 16:58
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

const userVotes = this.collectUserVotes();
const votes = countVotes(userVotes, this.props.mxEvent.getContent());
const totalVotes = this.totalVotes(votes);
const userId = MatrixClientPeg.get().getUserId();
Copy link
Member

Choose a reason for hiding this comment

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

can we use a MatrixClientContext here instead of MatrixClientPeg?

Copy link
Member Author

Choose a reason for hiding this comment

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

Something like 17b726c ? I've never used this before so please check I did it right :-)

Copy link
Member

Choose a reason for hiding this comment

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

Yup! This way will make any future transitions to a multi-account world much easier, thanks!

@andybalaam andybalaam merged commit 1c67033 into matrix-org:develop Nov 26, 2021
@andybalaam andybalaam deleted the psf-557-highlight-our-votes branch November 26, 2021 09:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

T-Task Refactoring, enabling or disabling functionality, other engineering tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants