-
Notifications
You must be signed in to change notification settings - Fork 793
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
Hide input component #1417
Hide input component #1417
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1417 +/- ##
========================================
Coverage 40.07% 40.07%
========================================
Files 465 465
Lines 13845 13845
========================================
Hits 5548 5548
Misses 8297 8297
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
monkey/monkey_island/cc/ui/src/components/ui-components/HideInput.js
Outdated
Show resolved
Hide resolved
this.setState({hidden: ! this.state.hidden}); | ||
} | ||
|
||
onChange(e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, my javascript is rusty. Does this also need a call to bind()
or equivalent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://newbedev.com/why-and-when-do-we-need-to-bind-functions-and-eventhandlers-in-react
Apparently there is couple ways of binding, in which I have use the third one.
onChange={(event) => this.onChange(event)}
monkey/monkey_island/cc/ui/src/styles/components/HideInput.scss
Outdated
Show resolved
Hide resolved
import {InputGroup, FormControl} from 'react-bootstrap'; | ||
import '../../styles/components/HideInput.scss' | ||
|
||
class HideInput extends React.PureComponent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a name like SensitiveTextInput
would be better? The input isn't always hidden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The develop branch has been backmerged into this branch. We need to undo/fix that before we merge this branch back into develop.
38d3296
to
2400979
Compare
What does this PR do?
Fixes #1183
Add any further explanations here.
PR Checklist
Was the documentation framework updated to reflect the changes?Testing Checklist
Added relevant unit tests?Explain Changes