search

BugiDev / react-native-calendar-strip

Easy to use and visually stunning calendar component for React Native.
MIT License
939 stars 327 forks source link

Return cloned moment dates #197

Closed rusticdeity closed 4 years ago

rusticdeity commented 4 years ago

I am returning cloned dates for onDateSelected, onWeekChanged and onHeaderSelected callbacks. I ran npm run-script format on the code. I think that is the reason for change in the formatting.

I have made changes in below files

  1. CalendarStrip.js:290,477
  2. Scroller.js:209
  3. CalendarHeader.js:90
peacechen commented 4 years ago

Thanks @GIT-Sachin for the PR.

There are a number of merge conflicts that need to be resolved. It looks like you merged master to your fork which should prevent that, but something's still not fully up to date in your fork.

The bulk of changes in commit f472e29 are formatting changes. We'll need to set the prettier config to not cause so many formatting issues. Please revert those as that makes it difficult to see that relevant functional changes. Edit: Upon researching this some more, I'm removing prettier since it doesn't respect maintaining multi-line props in JSX. There are issues open on its repo but they've been shot down.

The App.js similarly has a number of non-functional formatting changes, mostly changing indentation from 2 to 4 spaces. Please revert these as it also makes it hard to see the relevant changes.

Before committing, please run npm run eslint in the root project. It won't fix the line breaks introduced by the 80 character line limit of your IDE, but it checks for common syntactical issues.

peacechen commented 4 years ago

@GIT-Sachin Are these all the places you cloned the return dates?

I can commit those changes to master so you don't have to refactor this PR.

rusticdeity commented 4 years ago

@peacechen Yes that's right. I also specified the line numbers that i have made changes at. If you still want me to refactor the code I'll probably get time this weekend.

peacechen commented 4 years ago

I will pull out those changes and create a separate commit. Thanks for making this PR.