Skip to content

Conversation

@vatana7
Copy link
Contributor

@vatana7 vatana7 commented Aug 17, 2023

PR for #109

@invisal Please review and give feedback

Copy link
Collaborator

@invisal invisal left a comment

Choose a reason for hiding this comment

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

I recommend you to create another component called PasswordField.

In the TextField file, we can split the interface

export interface TextFieldCommonProps {
  readOnly?: boolean;
  onChange?: (value: string) => void;
  label?: string;
  value?: string;
  autoFocus?: boolean;
  placeholder?: string;
}

interface TextFieldProps extends TextFieldCommonProps  {
  multipleLine?: boolean;
  type?: 'text' | 'password';
  actionIcon?: ReactElement;
  actionClick?: () => void;
}

The common PasswordField can use the existing TextField. Maybe something like this

export default function PasswordField(props: TextFieldCommonProps) {
  // handle toggle logic here
  
  return <TextField {...props} actionIcon={...} actionClick={() => ...} />
  
}

For state, you don't need to use generic state as there is only one state at the moment.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vatana7
Copy link
Contributor Author

vatana7 commented Aug 17, 2023

@invisal i've changed the code as you suggested

Copy link
Collaborator

@invisal invisal left a comment

Choose a reason for hiding this comment

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

LGTM

@invisal invisal linked an issue Aug 18, 2023 that may be closed by this pull request
@invisal invisal changed the title feature: add toggle password visibility feat: add toggle password visibility Aug 18, 2023
@invisal invisal merged commit aa46097 into querymx:main Aug 18, 2023
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.

Feature Request: Toggle button to see password

2 participants