Skip to content

Fix issue #337, #339, #223 #345

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

Merged
merged 5 commits into from
Sep 1, 2017
Merged

Conversation

elfman
Copy link
Contributor

@elfman elfman commented Aug 27, 2017

#339 rely on #337 be fixed, so I rise PR for them together

@birdofpreyru
Copy link
Collaborator

@elfman Had no chance to test it carefully, though, during a brief try I've noted the following problem:

  • If I open desktop kind of menu on tablet (by tapping on, say, Learn) and then tap on the white part of the header (i.e. the same element where Compete, Learn and Community buttons are, just outside of them, and other elements, menu is not closed and there are some error in console.

@elfman
Copy link
Contributor Author

elfman commented Aug 28, 2017

Fixed, please test again.

@elfman
Copy link
Contributor Author

elfman commented Aug 28, 2017

issue #223 and #339 are both related with TopcoderHeader, so I put it in this PR too.
Please take it account when testing.

@elfman elfman changed the title Fix issue #337 and #339 Fix issue #337, #339, #223 Aug 28, 2017
@birdofpreyru
Copy link
Collaborator

birdofpreyru commented Aug 30, 2017

@elfman Wow :) These are big changes, and they seem to be working fine :)

There is just one more enhancement that would be handy: now if I use switch browser into touch device emulation mode, use touches to play with the menu, then switch to the normal mode and try to interact with the menu neither hovering nor clicks work anymore until I reload the app. I understand, that it is not an issue for production use, as user, in the same session, will not switch like this. Though, for dev/testing routine, it would be super-cool to fix it, so that such switches do not break anything. Also, I guess, it should be just a small modification somewhere, as there is no real need to disable all hover-related logic once you have opened a menu with touch, right?

@elfman
Copy link
Contributor Author

elfman commented Aug 30, 2017

@birdofpreyru MouseEnter & MouseLeave Event can also be triggered in mobile device. But their event logic is not fit to mobile device and can't work with touch event. For example, I tap on a menu button when menu opened, the menu should be closed as touch event do so, but the MouseEnter will also be trigger, and it will open the menu again, then the menu looks like nothing happened.
so I disable mouseenter and mouseleave when touch event triggered.
If you want to make it work when switching emulation mode, how about reset the isMobile state in onLayout when width changed?

@birdofpreyru
Copy link
Collaborator

@elfman No, I don't like listening the width change, as it probably will slow down window resizing (which is already not that fast). Can we just re-enable mouseenter and mouseleave once the menu is closed? I believe, it is already closed automatically if I switch between touch device and desktop in the way described above. And having them enabled when the menu is closed is not a problem for us?

@elfman
Copy link
Contributor Author

elfman commented Aug 30, 2017

That's a good idea, I'll do it

@elfman
Copy link
Contributor Author

elfman commented Aug 30, 2017

@birdofpreyru Updated.

@birdofpreyru birdofpreyru merged commit 154c434 into topcoder-platform:develop Sep 1, 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.

2 participants