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.

Sign up for GitHub

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

Jump to bottom

WIP: V2 release #36

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

WIP: V2 release #36

wants to merge 4 commits into from

Conversation

crup
Copy link
Contributor

@crup crup commented Apr 29, 2020 •
edited
Loading

Need suggestions:

Introducing breaking changes:

{time => <span>{time}</span>}

The children function will return an object instead of timer value.

The function will return start, stop and reset functions to manually start stop timer at any point of time.`

{
      value,
      start,
      stop,
      reset,
    }

This fixes enhancement request in #33

Action items:

  • Updated deps for security vulnerability warnings.
  • Updated timer with api described above.
  • Removed unused styles.css file in src
  • Update documentation
  • Update test cases
  • Refactor/clean timer component
  • Update examples
  • Introduce new prop onStart
  • Introduce support for React hooks

Hooks api:

const [value, start, stop, reset] = useReactTimer({
     start: 30,
     interval: 500,
     onTick: v => v-1,
});

JSX(with hooks)

<p>{value}</p>
<button type="button" onClick={start}>start</button>
<button type="button" onClick={stop}>stop</button>
<button type="button" onClick={reset}>reset</button>

JSX(without hooks)

{({
  value, reset, start, stop,
}) => (
  <div>
    {value}
    <div>
      <button type="button" onClick={start}>start</button>
      <button type="button" onClick={stop}>stop</button>
      <button type="button" onClick={reset}>reset</button>
    </div>
  </div>
)}

@crup crup requested a review from Bryze April 29, 2020 18:59
@crup crup self-assigned this Apr 29, 2020
@crup crup added enhancement New feature or request @next-release Functionality will be fixed/added in next release labels Apr 29, 2020
@crup crup linked an issue Apr 29, 2020 that may be closed by this pull request
How to restart the timer ? #33
Open
@Bryze
Copy link
Member

Bryze commented Apr 30, 2020 •
edited
Loading

We should introduce the breaking change as an opt-in for the next minor release while deprecating the old render props usage.

And then for the major release, we can move forward by removing it completely.

What are your thoughts?

Also, I'm on the opinion, we should use object propagation in all callback functions (onStart, onTick) and so if users opt-in they can change their implementation to be consistent with accepting objects in arguments

@crup
Copy link
Contributor Author

crup commented Apr 30, 2020

@Bryze Are you suggesting to go with a minor release which will support these features?

@Bryze
Copy link
Member

Bryze commented Apr 30, 2020

@Bryze Are you suggesting to go with a minor release which will support these features?

Yes, keeping it as an opt-in feature that will be disabled by default.

We'll then introduce the breaking change in the next major release. probably in the next couple of months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bryze Bryze Awaiting requested review from Bryze

Assignees

@crup crup

Labels
enhancement New feature or request @next-release Functionality will be fixed/added in next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

How to restart the timer ?
2 participants
@crup @Bryze