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 focus after cancelling modal #607

Merged
merged 3 commits into from
Oct 7, 2017
Merged

Conversation

wvds
Copy link

@wvds wvds commented Oct 1, 2017

Closes #446

Before submitting, please confirm you've

Please provide the following information:

Summary
After closing the modal the focus of the clicked link will be removed

Issue link
#446

this.props.onTeamRemove();
}
handleTeamEditing() {
document.activeElement.blur();
Copy link
Contributor

Choose a reason for hiding this comment

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

In the team list, TeamList.jsx controls modals. So I think it should call document.activeElement.blur().

@@ -9,9 +9,11 @@ class TeamListItem extends React.Component {
}

handleTeamRemove() {
document.activeElement.blur();
Copy link
Contributor

Choose a reason for hiding this comment

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

In the team list, TeamList.jsx controls modals. So I think it should call document.activeElement.blur().

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 2, 2017

Thanks @wvds, the behavior looks good! For better code, would you review my comments?

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 2, 2017

Copy link
Contributor

@jasonblais jasonblais left a comment

Choose a reason for hiding this comment

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

@wvds Also tested and works great. Big thanks for your contribution!

@yuya-oc just had minor comments. Would you like to help with them and it should be ready to merge?

@jasonblais jasonblais added this to the v3.8.0 milestone Oct 4, 2017
@wvds
Copy link
Author

wvds commented Oct 7, 2017

@jasonblais @yuya-oc I've made the changes to the pull request :-)
CI is also fixed: https://circleci.com/gh/mattermost/desktop/1052

@yuya-oc yuya-oc added the All Platforms null label Oct 7, 2017
@yuya-oc yuya-oc merged commit 85a1b34 into mattermost:master Oct 7, 2017
@wvds wvds deleted the GH-446 branch October 7, 2017 14:34
@jasonblais
Copy link
Contributor

Thanks @wvds!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants