Skip to content

Conversation

xkxd
Copy link
Contributor

@xkxd xkxd commented Mar 4, 2019

@Wikia/iwing

change base to IW-1022 before merging

@xkxd xkxd requested a review from vforge as a code owner March 4, 2019 06:25
@xkxd xkxd force-pushed the 100-nav-coverage branch from c3315f1 to 27fef26 Compare March 4, 2019 06:32
}
};

const onMouseWheel = (event) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need this handler anymore?

Copy link
Contributor Author

@xkxd xkxd Mar 4, 2019

Choose a reason for hiding this comment

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

probably not.

  1. mousewheel and DOMMouseScroll are non-standard => MDN suggests using wheel instead
  2. they were used to differently (don't know why) support mouse scrolling but it's already covered via scroll event. I've tried to reverse-engineer this code but didn't find any specific reasons to use it, so considering our current tech stack and moderns browsers I decided to remove it until we find some good reason to bring it back.

Copy link
Contributor

@jsutterfield jsutterfield left a comment

Choose a reason for hiding this comment

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

Just one question.

@xkxd xkxd merged commit 613fc84 into nav-fixes Mar 4, 2019
@xkxd xkxd deleted the 100-nav-coverage branch March 4, 2019 11:45
@xkxd xkxd restored the 100-nav-coverage branch March 4, 2019 11:46
@xkxd xkxd deleted the 100-nav-coverage branch March 4, 2019 11:47
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.

2 participants