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

Fix reset button on user info page #281

Merged
merged 3 commits into from Nov 12, 2017

Conversation

Projects
None yet
2 participants
@libre-man
Copy link
Collaborator

commented Nov 11, 2017

Description

This was done by stop using the bootstrap vue input buttons as they simply do not work as expected.

Fix reset button on user info page
This was done by stop using the bootstrap vue input buttons as they
simply do not work as expected.
@codecov

This comment has been minimized.

Copy link

commented Nov 11, 2017

Codecov Report

Merging #281 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #281   +/-   ##
=======================================
  Coverage   98.68%   98.68%           
=======================================
  Files          26       26           
  Lines        2880     2880           
=======================================
  Hits         2842     2842           
  Misses         38       38

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2ec734...326a686. Read the comment docs.

@libre-man libre-man requested a review from olmokramer Nov 12, 2017

@olmokramer
Copy link
Collaborator

left a comment

Please fix the indentation: you seem to have used an indentation of 2 in the new HTML where the rest of the page uses 4. We could also consider changing everything to 2 in the templates because even 4 is a bit much with all the nesting. But I'd rather do that in another PR and all at once.

Otherwise it seems to work fine.

</b-input-group>
</b-form-fieldset>

<b-form-fieldset v-if="canEditPw || canEditInfo">
<b-input-group left="Old Password">
<b-form-input :type="oldPwVisible ? 'text' : 'password'" v-model="oldPw"/>
<input v-if="oldPwVisible"
type="text"

This comment has been minimized.

Copy link
@olmokramer

olmokramer Nov 12, 2017

Collaborator

Is there any reason that we don't conditionally set the type anymore, but instead have two duplicate inputs with only a different type?

This comment has been minimized.

Copy link
@libre-man

libre-man Nov 12, 2017

Author Collaborator

because you cannot have a v-model and a conditional type according to vue.

This comment has been minimized.

Copy link
@olmokramer

olmokramer Nov 12, 2017

Collaborator

Ah ok didn't know that

@olmokramer olmokramer merged commit 8c1e8d2 into master Nov 12, 2017

4 checks passed

codecov/patch Coverage not affected when comparing b2ec734...326a686
Details
codecov/project 98.68% remains the same compared to b2ec734
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 98.681%
Details

@wafflebot wafflebot bot removed the in progress label Nov 12, 2017

@libre-man libre-man deleted the fix-userinfo-reset-button branch Nov 12, 2017

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.