Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

LoginPage #194

Merged
merged 18 commits into from
May 28, 2018
Merged

LoginPage #194

merged 18 commits into from
May 28, 2018

Conversation

jonthomp
Copy link
Contributor

Adds a reusable LoginPage component taken from the example site. Works well with libraries such as formik and uses a strings prop for editing displayed strings.

LoginPage is wrapped in a new HOC that helps when working with form field touched and error states. It'll be handy for similar components in the future too.

FormTextInput has had most of its internals removed as some are handled by React and props are now pretty much all imported and sent straight to Form.Input.

@jonthomp jonthomp requested a review from AaronCoplan May 27, 2018 16:39
@jonthomp jonthomp changed the title Account pages LoginPage May 27, 2018
Copy link
Contributor

@AaronCoplan AaronCoplan left a comment

Choose a reason for hiding this comment

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

Couple minor things, but looks good!

+onChange?: (SyntheticInputEvent<HTMLInputElement>) => void,
+onBlur?: (SyntheticInputEvent<HTMLInputElement>) => void,
...FormInputProps,
+label?: string,
|};

type State = {|
value: string | number,
|};

class FormTextInput extends React.PureComponent<Props, State> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are removing the state, let's make this a pure component

@@ -0,0 +1,13 @@
//@flow
const strings = {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the advantage of having this in its own file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly just to help reduce file length but also thought it might help at a later date if we decide to implement a more complex strings/translations system

@jonthomp jonthomp merged commit 188c5b1 into master May 28, 2018
@jonthomp jonthomp deleted the account-pages branch May 28, 2018 20:38
@jonthomp
Copy link
Contributor Author

jonthomp commented Jun 1, 2018

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants