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

[RFR] Introduce <PasswordInput> #3013

Merged
merged 2 commits into from Dec 11, 2019
Merged

[RFR] Introduce <PasswordInput> #3013

merged 2 commits into from Dec 11, 2019

Conversation

Kmaschta
Copy link
Contributor

@Kmaschta Kmaschta commented Mar 17, 2019

This PR introduce a new component in ra-ui-materialui, <PasswordInput>.
It is very similar to a <TextInput type="password">, but it adds a button to toggle visibility.

I found that it's a common need in our client projects, and it can be a great accessibility addition.

That being said, I'm willing to create a third-party library with this component if you think that this component doesn't have its place in ra-ui-materialui.

Also, I have another proposal: apply this toggle button directly on the TextInput if the type is password.

TODO

  • Implement the component
  • Update the documentation
  • Use it in the demo

Previews

image

image

image

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

So it seems you changed many files just because you run prettier on them. It makes it hard to tell the changes you made for the password input feature. Please revert the prettier changes on files you didn't modify for the password input feature - we'll fix the prettier problems in another PR.

Also, there hous not be a PasswordField, because the password should never be exposed.

docs/Inputs.md Outdated Show resolved Hide resolved
examples/demo/src/visitors/VisitorEdit.js Outdated Show resolved Hide resolved
examples/data-generator/src/customers.js Outdated Show resolved Hide resolved
examples/demo/src/visitors/VisitorCreate.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/Link.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/auth/Login.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/auth/LoginForm.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/auth/Logout.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/detail/TabbedShowLayout.js Outdated Show resolved Hide resolved
@djhi djhi added this to the 2.9.0 milestone Mar 22, 2019
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Again, you should simulate the hashing of the password in the server side, and the fact that a user update with no password doesn't empty the password in the server side. You can do all that in a wrapper around the data provider.

examples/demo/src/visitors/VisitorCreate.js Outdated Show resolved Hide resolved
examples/demo/src/visitors/VisitorEdit.js Show resolved Hide resolved
packages/ra-ui-materialui/src/input/PasswordInput.js Outdated Show resolved Hide resolved
@Kmaschta Kmaschta changed the title [RFR] Introduce <PasswordInput> [WIP] Introduce <PasswordInput> Apr 3, 2019
@fzaninotto
Copy link
Member

Any chance you can update that one soon?

@fzaninotto fzaninotto removed this from the 2.9.0 milestone Apr 25, 2019
@Kmaschta
Copy link
Contributor Author

Yeah, I think I'll just do it in plain JavaScript since none of the inputs are in TS and that I need to use a dependency.

@fzaninotto
Copy link
Member

OK, let's forget the TypeScript requirement

@Kmaschta Kmaschta changed the title [WIP] Introduce <PasswordInput> [RFR] Introduce <PasswordInput> May 3, 2019
@Kmaschta
Copy link
Contributor Author

Kmaschta commented May 3, 2019

OK, I replaced the direct password edition by a tab with two fields: password and confirm password, on both Create and Edit views.
But if I hash the password by changing the data provider, I fear that someone will use it as an example and it's not really a good practice.

Since the demo API is a false in-memory API with only purpose to show react admin features, are we forced to handle that password hashing?

PS: Screenshots have been updated on the first post.

@Luwangel
Copy link
Contributor

We use this version in one of our project. It's not TypeScript but there are hooks 😄

import React, { useState } from 'react';
import PropTypes from 'prop-types';
import { TextInput } from 'react-admin';

import IconButton from '@material-ui/core/IconButton';
import InputAdornment from '@material-ui/core/InputAdornment';
import Visibility from '@material-ui/icons/Visibility';
import VisibilityOff from '@material-ui/icons/VisibilityOff';

const PasswordInput = ({ defaultVisible, ...otherProps }) => {
    const [visible, setVisible] = useState(defaultVisible);

    return (
        <TextInput
            {...otherProps}
            type={visible ? 'text' : 'password'}
            InputProps={{
                endAdornment: (
                    <InputAdornment position="end">
                        <IconButton onClick={() => setVisible(!visible)}>
                            {visible ? <Visibility /> : <VisibilityOff />}
                        </IconButton>
                    </InputAdornment>
                ),
            }}
        />
    );
};

PasswordInput.propTypes = {
    defaultVisible: PropTypes.bool,
};

PasswordInput.defaultProps = {
    defaultVisible: false,
};

export default PasswordInput;

@Kmaschta
Copy link
Contributor Author

Is it worth it to rebase this one?
Or we don't need a <PasswordInput> in the core?
Or should I rewrite it entirely for the v3?

@fzaninotto
Copy link
Member

Yes, it's still worth rebasing! No need to rewrite it with hooks, but you're welcome to do so.

@Kmaschta Kmaschta force-pushed the password-input branch 5 times, most recently from b502f17 to a7058e1 Compare July 15, 2019 15:23
@Kmaschta
Copy link
Contributor Author

Rebased on next

@Kmaschta Kmaschta force-pushed the password-input branch 3 times, most recently from 8c8805c to 4ae3bdc Compare August 14, 2019 14:31
@Kmaschta
Copy link
Contributor Author

Done 👌

@djhi djhi added this to the 3.0.0 milestone Aug 14, 2019
examples/demo/src/i18n/en.js Show resolved Hide resolved
examples/demo/src/visitors/VisitorCreate.js Outdated Show resolved Hide resolved
examples/demo/src/visitors/VisitorEdit.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/PasswordInput.tsx Outdated Show resolved Hide resolved
@fzaninotto fzaninotto modified the milestones: 3.0.0, 3.1.0 Oct 3, 2019
@fzaninotto
Copy link
Member

Needs rebase

@fzaninotto fzaninotto removed this from the 3.1.0 milestone Nov 28, 2019
@Kmaschta Kmaschta added the RFR Ready For Review label Dec 10, 2019
@fzaninotto fzaninotto merged commit 3224878 into next Dec 11, 2019
@fzaninotto fzaninotto deleted the password-input branch December 11, 2019 08:43
@fzaninotto
Copy link
Member

Excellent, thanks!

@fzaninotto fzaninotto added this to the 3.1.0 milestone Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants