Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

Feat: Add Hide/show toggle feature for all password fields #1136

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sanjibansg
Copy link

@sanjibansg sanjibansg commented Sep 27, 2020

Description

Password toggle feature added to view the entered password to verify whether correct or not.

Fixes #1077

Type of Change:

  • Code
  • User Interface

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

Code/Quality Assurance Only

  • My changes generate no new warnings

How has this been tested

Feature GIF

Feature video

@codecov
Copy link

codecov bot commented Sep 27, 2020

Codecov Report

Merging #1136 into develop will increase coverage by 0.36%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1136      +/-   ##
===========================================
+ Coverage    86.93%   87.30%   +0.36%     
===========================================
  Files           85       85              
  Lines         4057     4057              
  Branches       237      237              
===========================================
+ Hits          3527     3542      +15     
+ Misses         458      445      -13     
+ Partials        72       70       -2     
Impacted Files Coverage Δ
vms/event/views.py 79.32% <0.00%> (+1.68%) ⬆️
vms/shift/views.py 63.60% <0.00%> (+2.20%) ⬆️

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 0ab16d3...f0641ef. Read the comment docs.

Copy link

@Kajol-Kumari Kajol-Kumari left a comment

Choose a reason for hiding this comment

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

@Kahanikaar please attach the mock/GIF of the feature in the PR description.

@sanjibansg
Copy link
Author

@Kahanikaar please attach the mock/GIF of the feature in the PR description.

Screenshots of the feature added. Will this work? @Kajol-Kumari


var input = $("#id_password");
input.attr('type') === 'password' ? input.attr('type','text') : input.attr('type','password')
});

Choose a reason for hiding this comment

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

please leave a blank line here.

Copy link
Author

Choose a reason for hiding this comment

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

Changes are made as per asked. Do check @Kajol-Kumari

@satya7289
Copy link
Contributor

@Kahanikaar please update your description of the PR, Put the gif/ screenshot or video showing how this feature is working, in the How has this been tested section.
Also, please follow the commit message guidelines otherwise, it would not get merge.(also squash multiple commits into the single commit)

@Kajol-Kumari Kajol-Kumari added the Status: Needs Review PR needs an additional review or a maintainer's review. label Oct 4, 2020
Kajol-Kumari
Kajol-Kumari previously approved these changes Oct 4, 2020
Copy link

@Kajol-Kumari Kajol-Kumari left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Please squash the commits so that It can be taken forward for merging.

@sanjibansg
Copy link
Author

LGTM +1
Please squash the commits so that It can be taken forward for merging.

Squashed the commits and changed the commit message. Do review. @Kajol-Kumari @satya7289

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Add Hide/show toggle feature for all password fields
3 participants