Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Rework menus for mobile compatibility #950

Merged
merged 6 commits into from Jul 13, 2015

Conversation

Projects
None yet
2 participants
Contributor

saivann commented Jul 7, 2015

(This pull request replaces #931 and #932)

Live preview: http://bitcointest4.us.to/en/choose-your-wallet

This is an important re-design of the underlying javascript functions and CSS stylesheets used for navigation menus (top menu and wallet menu).

This pull request should fix several navigation bugs and issues for iOS, as well as speeding up navigation and preventing accidental clicks on Android, by making good use of the touch events.

The dropdown menus on mobile devices also now avoid using CSS :hover, and can be expanded or shrinked by clicking on the parent element of any menu entry. This avoids confusion and accidental clicks by moving elements on the screen only on user request.

The wallets are now also populated, displayed and hidden on click and not on mouseover, which prevents accidentally closing a wallet bubble only because the pointer of the mouse slipped out of the bubble, and allows to shorten and simplify javascript code quite a lot. This also fixes a bug where a wallet would inconsistently display on mouseover after user clicked inside its bubble.

Remaining changes include improving the syntax and comments for the related CSS and javascript code, as well as fixing a few accidental use of global variables.

Tested on OP 9,12, FF3.5,38, SA 4, IE 6-10, CH 43, Android 2.3.6
Tested English and Arabic RTL layout
Tested website with javascript disabled

Since this is a massive pull request, I do not expect someone to fully review the code before merge. More testing of the live preview above is very appreciated, especially on iOS devices.

Rework menus for mobile compatibility
Start supporting touch events for non-buggy iOS support and faster menus
Display and hide wallets on click instead of on mouseout to prevent accidental clicks or mouseout events
Add a wrapper function to detect ghost and accidental clicks
Reorganize wallet javascript functions for better readability
Fix bug in which a wallet would display on mouse over after user clicked inside it's bubble
Fix accidental use of global variables

saivann added some commits Jul 7, 2015

Contributor

saivann commented Jul 8, 2015

In the absence of critical feedback, this pull request will be merged on July 13th.

Contributor

harding commented Jul 8, 2015

Testing this out on a phone, it looks like the only options are Mobile, Desktop, Hardware, and Web:

screen shot 2015-07-04 at 12 19 09

I wonder if it would be better to default to opening the Mobile menu on mobile devices:

screen shot 2015-07-04 at 12 17 25

That way visitors know they can filter down to their particular device.

I realize that this isn't really a comment on the code changes in this PR---it just happened to occur to me while reviewing since I don't usually visit this page---so let me know if I should open a separate issue or my own PR about it.

Contributor

harding commented Jul 8, 2015

Note to any other reviewers: changing languages doesn't work on @saivann's preview, but it does work for me on a local build. I think for the preview @saivann just did a quick build without any of the translations enabled.

Contributor

saivann commented Jul 8, 2015

@harding Thanks, that makes sense! I've just added a commit and updated the live preview.

Re translations enabled: Indeed, I've enabled English and Arabic translations only for that live preview to save time. This should be enough to test both normal and RTL layout, but I can include more languages if needed.

Contributor

harding commented Jul 8, 2015

LGTM 3ac6158. Reviewed code and moderately tested using FF on desktop and an ancient Android phone. Thanks!

@saivann saivann merged commit 1bd4720 into bitcoin-dot-org:master Jul 13, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@saivann saivann deleted the saivann:jsmobilewalletclick branch Jul 13, 2015

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