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

Implement final UI of hw wallet loading screen - Closes #1933 #1969

Merged
merged 10 commits into from Apr 30, 2019

Conversation

Projects
None yet
2 participants
@slaweet
Copy link
Member

commented Apr 30, 2019

What issue have I solved?

#1933

How have I implemented/fixed it?

Work in progress

How has this been tested?

  1. Go to https://ci.lisk.io/test/lisk-hub/PR-1969/#/hw-walet-login-v2
  2. See the loading screen

Or locally
npm run build-electron && LISK_HUB_URL="https://ci.lisk.io/test/lisk-hub/PR-1969/" npm run start
to test it including connecting the device

Review checklist

@slaweet slaweet self-assigned this Apr 30, 2019

@slaweet slaweet force-pushed the 1933-hardware-wallet-loading-screen branch from b346c38 to 2e0758e Apr 30, 2019

@slaweet slaweet force-pushed the 1933-hardware-wallet-loading-screen branch from 2e0758e to 7af57d8 Apr 30, 2019

slaweet added some commits Apr 30, 2019

@slaweet slaweet marked this pull request as ready for review Apr 30, 2019

@slaweet slaweet requested a review from massao Apr 30, 2019

slaweet added some commits Apr 30, 2019

Ignore test coverage of hwWalletLogin parts
... that are HOC or not yet implemented
@massao

massao approved these changes Apr 30, 2019

@massao
Copy link
Contributor

left a comment

just some small comments

wrapper = mount(<HwWalletLogin {...props}/>, options);
wrapper.update();
// TODO figure out why the assertion below doesn't work
// expect(wrapper.text()).toContain('Looking for a device...');

This comment has been minimized.

Copy link
@massao

massao Apr 30, 2019

Contributor

the toContain checks if an item on the array is the expected value. You can use toIncludeText in this case

This comment has been minimized.

Copy link
@slaweet

slaweet Apr 30, 2019

Author Member

Good point about toContain, I changed it, but use expect.stringContaining and kept it commented out since the test still doesn't work. I guess it's something with MultiStep using React.cloneElement

});
wrapper.update();
// TODO figure out why the assertion below doesn't work
// expect(wrapper.text()).toContain('SelectDevice');

This comment has been minimized.

Copy link
@massao

massao Apr 30, 2019

Contributor

Same here

slaweet added some commits Apr 30, 2019

@massao

massao approved these changes Apr 30, 2019

@slaweet slaweet merged commit 0e40a15 into development Apr 30, 2019

4 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
security/snyk - package.json (LiskHQ) No new issues
Details

@slaweet slaweet added the ready label Apr 30, 2019

@slaweet slaweet deleted the 1933-hardware-wallet-loading-screen branch May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.