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

Wallets: fix accidental wallet hiding on category change #1594

Merged
merged 1 commit into from May 10, 2017

Conversation

Projects
None yet
3 participants
Contributor

harding commented May 10, 2017

Closes #1493

Preview: http://dg1.dtrt.org/en/choose-your-wallet

Currently when you load the page in a mobile browser and click on a wallet, a copy of that that wallet's entry is copied using JS to a box later in the page where you can read about the wallet details. When you change platform, all the currently displayed wallet icons are cleared and then refilled with the icons for the newly-selected platform, with a check running that ensure each wallet is only displayed once (which is important for combination categories such as "mobile" where we don't want to show the same wallet twice just because it has an Android and iOS version).

Since the wallet details box in the mobile display wasn't cleared when you changed platform, the check that prevents duplicate wallet displays thought that wallet had already been included in the listing and so prevented it from being displayed again.

This patch fixes the problem by giving the copy of the wallet entry that goes in the wallet details box its own id, which seems to have resolved the issue and has no side effects I've seen.

I've tested on Firefox 45 ESR for Linux and Chromium for Linux, both in mobile view, and on Chrome for Android on a device. More testing always welcome.

Thanks to @crwatkins who first reported this issue.

Wallets: fix accidental wallet hiding on category change
Closes #1493

When a wallet was selected on mobile displays, its entry was copied to
another part of the page with its element id intact.  When a category
was changed, it prevented duplicate entries from being shown by
searching the page for matching ids, which unintentionally matched the
copied element.

This patch changes the id of the copied entry during the copy operation.
Contributor

crwatkins commented May 10, 2017

ACK. Tested on iOS in Chrome and Safari.

@wbnns wbnns merged commit 95c7a19 into bitcoin-dot-org:master May 10, 2017

1 check passed

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

wbnns commented May 10, 2017

Thanks guys!

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