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

Implement button to remove a room from a group #1438

Merged
merged 4 commits into from Oct 3, 2017

Conversation

lukebarnard1
Copy link
Contributor

@lukebarnard1 lukebarnard1 commented Sep 29, 2017

NB: This doesn't provide any feedback to the user. We should use a GroupSummaryStore-style component to refresh the view after a successful hit to the API. This could affect the summary view as well, because when rooms are removed from a group, they are also removed from the summary (if necessary).

CSS at element-hq/element-web#5141

NB: This doesn't provide any feedback to the user. We should use a GroupSummaryStore-style component to refresh the view after a successful hit to the API. This could affect the summary view as well, because when rooms are removed from a group, they are also removed from the summary (if necessary).
e.preventDefault();
e.stopPropagation();
this.context.matrixClient
.removeRoomFromGroup(this.props.groupId, this.props.groupRoom.roomId);
Copy link
Member

Choose a reason for hiding this comment

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

We should probably get into the habit generally of catching errors & popping up dialogs for them, unless the plan was to get the store to do this once it was written?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did forget to catch and put up a dialog. I'm not sure if we can get away with the store doing it, because if we follow the same pattern as GroupSummaryStore, the store would emit an error event. I can see this being annoying if we want to do something following an error in the promise chain.

(Tangentially related: I don't like error dialogs, they're annoying to click away)

onDeleteClick: function(e) {
e.preventDefault();
e.stopPropagation();
this.context.matrixClient
Copy link
Member

Choose a reason for hiding this comment

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

This will need a confirmation dialog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, agreed

@lukebarnard1 lukebarnard1 merged commit 03581ad into develop Oct 3, 2017
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.

None yet

2 participants