Skip to content
This repository has been archived by the owner on Jul 28, 2020. It is now read-only.

[CurrencyInput] convert to hooks #887

Merged
merged 2 commits into from
Feb 19, 2019
Merged

[CurrencyInput] convert to hooks #887

merged 2 commits into from
Feb 19, 2019

Conversation

mahmoud-zakria-sociomantic
Copy link
Contributor

For #886

@conor-cafferkey-sociomantic conor-cafferkey-sociomantic mentioned this pull request Feb 15, 2019
Closed
35 tasks
daniel-martic-sociomantic
daniel-martic-sociomantic reviewed Feb 18, 2019
View reviewed changes
Copy link
Contributor

@daniel-martic-sociomantic daniel-martic-sociomantic left a comment

Choose a reason for hiding this comment

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

Good job! Besides this suggestions, looks and works nicely 👍 :

  • Instead of patt, I would suggest to rename this const to pattern: it's easier to understand and doesn't "cost" many more characters than "patt" 😄
  • Few minor suggestions regarding empty lines:

src/CurrencyInput/index.jsx Show resolved Hide resolved
src/CurrencyInput/index.jsx Outdated Show resolved Hide resolved
daniel-martic-sociomantic
daniel-martic-sociomantic approved these changes Feb 19, 2019
View reviewed changes
Copy link
Contributor

@daniel-martic-sociomantic daniel-martic-sociomantic left a comment

Choose a reason for hiding this comment

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

🚀

damian-rodriguez-sociomantic
damian-rodriguez-sociomantic approved these changes Feb 19, 2019
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@damian-rodriguez-sociomantic damian-rodriguez-sociomantic merged commit dd3a327 into Milestone-Hooks! Feb 19, 2019
@damian-rodriguez-sociomantic damian-rodriguez-sociomantic deleted the Issue-886-CurrencyInput-hooks branch February 19, 2019 17:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@daniel-martic-sociomantic daniel-martic-sociomantic daniel-martic-sociomantic approved these changes

@damian-rodriguez-sociomantic damian-rodriguez-sociomantic damian-rodriguez-sociomantic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mahmoud-zakria-sociomantic @damian-rodriguez-sociomantic @daniel-martic-sociomantic