db-console: convert email subscription and input components to functional components#164287
Conversation
|
😎 Merged successfully - details. |
|
This change is |
jasonlmfong
force-pushed
the
jf/console-login-func
branch
2 times, most recently
from
a4f0c95 to
b751154
Compare
There was a problem hiding this comment.
this one looks like a lot less but it's actually faithful
| import "./emailSubscription.scss"; | ||
|
|
||
| type EmailSubscriptionProps = MapDispatchToProps & MapStateToProps; | ||
| const EmailSubscription: React.FC = () => { |
There was a problem hiding this comment.
Just curious: how come some components are typed with React.FC (like this one) and others arent (like the TextInput Above)
There was a problem hiding this comment.
no particular reason, i can make them unified
| loginState, | ||
| handleLogin, | ||
| }) => { | ||
| // TODO(vilterp): focus username field on mount |
| export class TextInput extends React.Component<TextInputProps, TextInputState> { | ||
| static defaultProps = { | ||
| initialValue: "", | ||
| validate: () => false, |
There was a problem hiding this comment.
I don't get why TS doesn't have an issue with this since it clearly doesn't match the type of the validate filed in TextInputProps.
There was a problem hiding this comment.
static defaultProps wasn't typechecked unfortunately
| ); | ||
| }; | ||
|
|
||
| const LoginPageConnected: React.FC<RouteComponentProps> = ({ |
There was a problem hiding this comment.
Is this wrapper component actually needed? Can we just pass the LoginPage component to the router directly? Also there is a location prop on the RouteComponentProps` object, so I would think that you could use that instead of the location object stored in the router reducer. Do you know if there is a reason for this?
There was a problem hiding this comment.
state.router.location is from redux so i imagine that was a convenience thing, i will use the location in the new version
as for removing the wrapper i left it alone because i just wanted to do the functional change
Convert the following class components to functional style:
- PasswordInput (components/input/passwordInput.tsx):
1. Replace `showPassword` class state with `useState`
2. Inline `renderPasswordIcon` helper into JSX
3. Convert `handleOnTextChange` and `togglePassword` to local functions
- TextInput (components/input/textInput.tsx):
1. Replace 5 class state vars with individual `useState` hooks
2. Replace `static defaultProps` with default parameter values
3. Convert `validateInput`, `handleOnTextChange`, `handleOnBlur` to local functions
Epic: CRDB-58145
Release note: None
Convert the following class components to functional style:
- EmailSubscription:
1. connect → useSelector + useDispatch
2. componentDidMount/componentWillUnmount → useEffect with cleanup return
Epic: CRDB-58145
Release note: None
jasonlmfong
force-pushed
the
jf/console-login-func
branch
from
b751154 to
2204662
Compare
|
split out the actual login changes to #164534 |
jasonlmfong
changed the title
|
/trunk merge TFTR! |
celeste-cockroachdb
bot
added
v26.2.0-prerelease
and removed
target-release-26.2.0
labels
3 participants
Convert the following class components to functional style:
showPasswordclass state withuseStaterenderPasswordIconhelper into JSXhandleOnTextChangeandtogglePasswordto local functionsuseStatehooksstatic defaultPropswith default parameter valuesvalidateInput,handleOnTextChange,handleOnBlurto local functionsEpic: CRDB-58145
Release note: None