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

Add confirmation dialog to kick/ban buttons #694

Merged
merged 6 commits into from
Feb 14, 2017
Merged

Add confirmation dialog to kick/ban buttons #694

merged 6 commits into from
Feb 14, 2017

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Feb 14, 2017

Add a specific dialog used for confirming member actions.

Also remove onFinished from MemberInfo which did absolutely
nothing.

Requires element-hq/element-web#3224
Fixes element-hq/element-web#3218

Add a specific dialog used for confirming member actions.

Also remove onFinished from MemberInfo which did absolutely
nothing.
@lukebarnard1
Copy link
Contributor

On

<Element {...props} onFinished={closeMenu}/>
, onFinished is used... albeit in a very subtle way.

I suppose

// var MemberInfo = sdk.getComponent('rooms.MemberInfo');
is commented so you could argue that it's really not being used, but then I would also remove that commented stuff for good.

/*
* A dialog for confirming an operation on another user.
* Takes a user ID and a verb, displays the target user prominently
* such that it should be easy to confirm that tne operation is being
Copy link
Contributor

Choose a reason for hiding this comment

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

the

export default React.createClass({
displayName: 'ConfirmUserActionDialog',
propTypes: {
member: React.PropTypes.object.isRequired, // member object
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use PropTypes.shape to indicate what properties member should have.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought about it, although it's a standard JS SDK member object.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh ok.. I wished there was a thing for this but I don't think there is... maybe change the comment so that it's obvious that member is a RoomMember?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh.. you did that, in that case LGTM

dbkr added 2 commits February 14, 2017 16:09
That I've now broken such that it wouldnt work if it were
uncommented
@dbkr
Copy link
Member Author

dbkr commented Feb 14, 2017

Oh, good spot. I've removed the commented out stuff.

Copy link
Contributor

@lukebarnard1 lukebarnard1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@lukebarnard1 lukebarnard1 assigned dbkr and unassigned lukebarnard1 Feb 14, 2017
@dbkr dbkr merged commit 973b92b into develop Feb 14, 2017
@richvdh richvdh deleted the dbkr/confirm_ban branch February 15, 2017 13:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants