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

fix(StatusAlert): Fix focus error for dismissible alert. #89

Merged
merged 1 commit into from
Dec 4, 2017

Conversation

MichaelRoytman
Copy link
Contributor

StatusAlert requires a check for existence of xButton in componentDidUpdate to prevent errors when
displaying dismissible alerts.

StatusAlert requires a check for existence of xButton in componentDidUpdate to prevent errors when
displaying dismissible alerts.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 99.506% when pulling 4ee3743 on mroytman/dismissible-status-alert-xButton into 25fe397 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 99.506% when pulling 4ee3743 on mroytman/dismissible-status-alert-xButton into 25fe397 on master.

@jaebradley
Copy link
Contributor

Are there any additional tests to add so that we know if we change the component's behavior going forward?

@MichaelRoytman
Copy link
Contributor Author

@jaebradley I think that, if I were to add testing for validation of focus-related behavior, I'd want to add test for focus logic in the StatusAlert as a whole, and I think it might be out-of-scope for me at the moment. I'm inclined to leave that until a later date, since the coverage is essentially unchanged. I'll make a note of it, though. Thank you for the review!

@MichaelRoytman MichaelRoytman merged commit 8a12d91 into master Dec 4, 2017
@MichaelRoytman MichaelRoytman deleted the mroytman/dismissible-status-alert-xButton branch December 4, 2017 15:29
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

4 participants