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

Fabo/1835 signing methods #1860

Merged
merged 20 commits into from
Jan 25, 2019
Merged

Fabo/1835 signing methods #1860

merged 20 commits into from
Jan 25, 2019

Conversation

faboweb
Copy link
Collaborator

@faboweb faboweb commented Jan 22, 2019

Closes #1835

Description:

image

Missing:

  • tests

❤️ Thank you!


  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer

@jbibla
Copy link
Collaborator

jbibla commented Jan 23, 2019

please don't include that line in the form below amount.

@faboweb
Copy link
Collaborator Author

faboweb commented Jan 23, 2019

please don't include that line in the form below amount.

which line?

@faboweb
Copy link
Collaborator Author

faboweb commented Jan 23, 2019

ahhh, ok

@codecov
Copy link

codecov bot commented Jan 23, 2019

Codecov Report

Merging #1860 into develop will decrease coverage by 0.57%.
The diff coverage is 93.49%.

@@             Coverage Diff             @@
##           develop    #1860      +/-   ##
===========================================
- Coverage    95.08%   94.51%   -0.58%     
===========================================
  Files          124      125       +1     
  Lines         2910     2916       +6     
  Branches       121      117       -4     
===========================================
- Hits          2767     2756      -11     
- Misses         135      150      +15     
- Partials         8       10       +2
Impacted Files Coverage Δ
.../renderer/components/common/TmConnectedNetwork.vue 90% <ø> (ø) ⬆️
...src/renderer/components/common/TmSessionSignIn.vue 90.32% <ø> (ø) ⬆️
app/src/renderer/components/common/TmFormGroup.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/ShortBech32.vue 100% <ø> (ø) ⬆️
app/src/renderer/App.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/ToolBar.vue 91.66% <ø> (ø) ⬆️
...pp/src/renderer/components/common/TmPageHeader.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/AppHeader.vue 93.75% <ø> (ø) ⬆️
...pp/src/renderer/components/staking/LiValidator.vue 96% <ø> (ø) ⬆️
app/src/renderer/components/common/TmBalance.vue 85.71% <ø> (ø) ⬆️
... and 30 more

@codecov
Copy link

codecov bot commented Jan 23, 2019

Codecov Report

Merging #1860 into develop will decrease coverage by 0.53%.
The diff coverage is 94.35%.

@@             Coverage Diff             @@
##           develop    #1860      +/-   ##
===========================================
- Coverage    95.08%   94.54%   -0.54%     
===========================================
  Files          124      125       +1     
  Lines         2910     2916       +6     
  Branches       121      117       -4     
===========================================
- Hits          2767     2757      -10     
- Misses         135      150      +15     
- Partials         8        9       +1
Impacted Files Coverage Δ
.../renderer/components/common/TmConnectedNetwork.vue 90% <ø> (ø) ⬆️
...src/renderer/components/common/TmSessionSignIn.vue 90.32% <ø> (ø) ⬆️
app/src/renderer/components/common/TmFormGroup.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/ShortBech32.vue 100% <ø> (ø) ⬆️
app/src/renderer/App.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/ToolBar.vue 91.66% <ø> (ø) ⬆️
...pp/src/renderer/components/common/TmPageHeader.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/AppHeader.vue 93.75% <ø> (ø) ⬆️
...pp/src/renderer/components/staking/LiValidator.vue 96% <ø> (ø) ⬆️
app/src/renderer/components/common/TmBalance.vue 85.71% <ø> (ø) ⬆️
... and 30 more

@codecov
Copy link

codecov bot commented Jan 24, 2019

Codecov Report

Merging #1860 into develop will decrease coverage by 0.01%.
The diff coverage is 95.23%.

@@             Coverage Diff             @@
##           develop    #1860      +/-   ##
===========================================
- Coverage    94.97%   94.95%   -0.02%     
===========================================
  Files          125      125              
  Lines         2905     2896       -9     
  Branches       117      117              
===========================================
- Hits          2759     2750       -9     
- Misses         135      137       +2     
+ Partials        11        9       -2
Impacted Files Coverage Δ
...pp/src/renderer/components/staking/LiValidator.vue 96% <ø> (ø) ⬆️
.../src/renderer/components/staking/PageValidator.vue 100% <100%> (ø) ⬆️
...rc/renderer/components/governance/PageProposal.vue 100% <100%> (ø) ⬆️
app/src/renderer/components/wallet/PageWallet.vue 100% <100%> (ø) ⬆️
...p/src/renderer/components/governance/ModalVote.vue 100% <100%> (ø) ⬆️
.../renderer/components/staking/UndelegationModal.vue 100% <100%> (ø) ⬆️
.../renderer/components/governance/PageGovernance.vue 100% <100%> (ø) ⬆️
...rc/renderer/components/governance/ModalPropose.vue 100% <100%> (+3.44%) ⬆️
app/src/renderer/components/common/ActionModal.vue 100% <100%> (ø) ⬆️
...rc/renderer/components/staking/DelegationModal.vue 100% <100%> (ø) ⬆️
... and 2 more

@faboweb faboweb changed the title [WIP] Fabo/1835 signing methods Fabo/1835 signing methods Jan 24, 2019
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

untested review comments:

app/src/renderer/components/staking/DelegationModal.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/ModalVote.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/ModalPropose.vue Outdated Show resolved Hide resolved
app/src/renderer/components/staking/DelegationModal.vue Outdated Show resolved Hide resolved
app/src/renderer/components/staking/UndelegationModal.vue Outdated Show resolved Hide resolved
app/src/renderer/components/wallet/SendModal.vue Outdated Show resolved Hide resolved
app/src/renderer/components/wallet/SendModal.vue Outdated Show resolved Hide resolved
app/src/renderer/components/wallet/SendModal.vue Outdated Show resolved Hide resolved
fedekunze
fedekunze previously approved these changes Jan 24, 2019
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM

this.sending = true
this.$v.$touch()

let subFormValid = this.validate()
Copy link
Collaborator

@jbibla jbibla Jan 25, 2019

Choose a reason for hiding this comment

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

what's a subForm? and what is this.validate()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The actual form like SendModal. Added a bit of explanation.

Copy link
Collaborator

@jbibla jbibla left a comment

Choose a reason for hiding this comment

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

few small comments - requesting changes because @fedekunze already approved - but this is an amazing next step for ActionModals!


// An ActionModal is only the prototype of a parent modal
// here we trigger the validation of the form that this parent modal
let childFormValid = this.validate()
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice rename. thank you!

@jbibla jbibla merged commit 70746e2 into develop Jan 25, 2019
@faboweb faboweb deleted the fabo/1835-signing-methods branch January 25, 2019 17:48
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.

Allow for different signing mechanisms
3 participants