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

feat: Implement miles/kilometer selector #252

Merged
merged 11 commits into from
Oct 6, 2019

Conversation

matepapp
Copy link
Contributor

@matepapp matepapp commented Oct 5, 2019

Resolves #244

Prettier

I tried to infer the project's code formatting rules, but I couldn't figure it out completely, that's why I additionally created a prettierrc to configure prettier. If it's too much, please comment on it and I'll remove.

Ohh I've just found an issue why you might didn't want to use prettier at the first case. I removed the prettier config and sorry if I messed up your formatting 😶

Screen recording

Image 2

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

Thanks @matepapp for this wonderful PR! It works perfectly.

I just added some comments on the code. 95% are just nits, I tend to order stuff alphabetically if I don't see any other more logical ordering.

And you're right about linting, sorry about that, Prettier is a good idea. And those 95% of nits above should be definitely enforced by a linter. I created an issue for that #253

App/App.tsx Outdated Show resolved Hide resolved
App/Screens/Home/Header/Header.tsx Outdated Show resolved Hide resolved
App/Screens/Home/Header/Header.tsx Outdated Show resolved Hide resolved
App/Screens/About/About.tsx Outdated Show resolved Hide resolved
App/stores/distanceUnit.tsx Outdated Show resolved Hide resolved
DistanceUnit
} from '../../util/station';
import { distanceToStation, getCorrectLatLng } from '../../util/station';
import { useDistanceUnit } from '../../stores/distanceUnit';
Copy link
Member

Choose a reason for hiding this comment

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

alphabetical

App/stores/distanceUnit.tsx Outdated Show resolved Hide resolved
App/stores/distanceUnit.tsx Outdated Show resolved Hide resolved
App/stores/distanceUnit.tsx Outdated Show resolved Hide resolved
App/Screens/About/About.tsx Outdated Show resolved Hide resolved
@amaury1093 amaury1093 changed the title Implement miles/kilometer selector feat: Implement miles/kilometer selector Oct 6, 2019
@matepapp
Copy link
Contributor Author

matepapp commented Oct 6, 2019

No problem, thanks for the comments 🙌 I think I fixed everything but double check it please.

Copy link
Member

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

Perfect!

One last thing: the CI shouts about some linting issues. Running yarn lint --fix once should solve all of them.

Thanks again.

@amaury1093 amaury1093 merged commit ca91f5b into shootismoke:master Oct 6, 2019
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.

Allow users to change between miles and km
3 participants