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

Remove userId property on RightPanel #4775

Merged
merged 2 commits into from
Aug 11, 2017
Merged

Conversation

lukebarnard1
Copy link
Contributor

because we shouldn't have a dispatch AND have a property that do vaguely similar things. Ideally, the dispatch would send a userId and RP would do async work to get the member/avatar/displayname.

because we shouldn't have a dispatch AND have a property that do vaguely similar things. Ideally, the dispatch would send a userId and RP would do async work to get the member/avatar/displayname.
@@ -31,7 +31,6 @@ module.exports = React.createClass({
displayName: 'RightPanel',

propTypes: {
userId: React.PropTypes.string, // if showing an orphaned MemberInfo page, this is set
Copy link
Member

Choose a reason for hiding this comment

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

Presumably the long term plan here to do the same for roomId, in which case we should add a comment or else someone will change it back to using props again.

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 Aug 10, 2017

Choose a reason for hiding this comment

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

I don't think so given that the file panel and member list take the room as a prop. I suppose the RightPanel could listen to view_room instead of accepting roomID as a prop?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm assuming we'd be consistent and eventually move to using dispatches everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh I mean it should really listen to the RVS like everything else. I shall comment

@dbkr dbkr assigned lukebarnard1 and unassigned dbkr and lukebarnard1 Aug 10, 2017
@lukebarnard1 lukebarnard1 assigned dbkr and unassigned lukebarnard1 Aug 10, 2017
@dbkr dbkr merged commit 3eeabe8 into develop Aug 11, 2017
@t3chguy t3chguy deleted the luke/fix-user-url-no-middle-panel branch May 12, 2022 08:59
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

Successfully merging this pull request may close these issues.

2 participants