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

Restore original styles when removing hover #5570

Merged
merged 30 commits into from Jun 26, 2018
Merged

Conversation

benmccann
Copy link
Contributor

@benmccann benmccann commented Jun 16, 2018

@loicbourgois sent #5194 which was closed due to inactivity. This is an updated version of the original PR. I've made just a minor change so that the tests pass

This PR stores the original styles when hovering and then restores them when removing the hover.

This reduces the amount of code used for applying and removing hover styles by a fair amount (though the PR adds a lot of test code, so the overall lines of code don't change much)

This also fixes hovering on the financial charts. You can see that the financial chart samples are broken. Hovering applies a different style, but removing hover does not restore them.

@benmccann benmccann force-pushed the hover branch 4 times, most recently from 61a472e to 30380c4 Compare June 16, 2018 04:14
@@ -19,7 +19,6 @@ module.exports = function(karma) {
// These settings deal with browser disconnects. We had seen test flakiness from Firefox
// [Firefox 56.0.0 (Linux 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.
// https://github.com/jasmine/jasmine/issues/1327#issuecomment-332939551
browserNoActivityTimeout: 60000,
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 removed this line that I had added in an earlier PR because it's very annoying to use with gulp test --watch. It closes the browser after 10 min which makes debugging really hard. Hopefully browserDisconnectTolerance: 3 will be enough. We could make it browserDisconnectTolerance: 5 or something if we want to add a little extra buffer since we're removing browserNoActivityTimeout

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.

None yet

4 participants