React-calendar-timeline: Item re-selection requires multiple clicks

[hotyhuang]

Hi,

I am trying with the simplest usage, with 3 items. I first selected "item 1", then I selected "item 2". But afterwards, when I trying click back on "item 1", it is not selected. I even tried with onItemSelect, it is not catching the later event.

Is this a known issue?

[mcMickJuice]

hi @hotyhuang can you please provide a codesandbox link that reproduces this issue? Just fork one of the examples

[hotyhuang]

hi @mcMickJuice , you can checkout this most simple one:
https://codesandbox.io/s/94k78m50xw

Each item cannot be clicked 2nd time and get highlighted.

[mcMickJuice]

Hey @hotyhuang im not able to reproduce your issue. Couple of questions:

• what browser are you using?
• are you using a touch device?
• if no touch device, are you using a mouse or a trackpad to make these clicks?

[hotyhuang]

@mcMickJuice I am using laptop, mouse clicks. I just tested a few, think it happens in certain situation. So here is several cases I tried:

• win7, Chrome (v65.0), re-select needs multiple clicks, unstable

• win7, Chrome (v66.0), re-select needs multiple clicks, unstable

• win7, Firefox (v56.0), works perfectly

• mac, Chrome (v65.0) + FF + Safari, works perfectly

I thought Chrome is fine for most cases, so didn't realize it performs diff across browsers & systems.

[mcMickJuice]

Thanks for putting in the effort to test this out @hotyhuang !

User interaction management in this library is suboptimal. Is it reasonable to change the title of this issue to "Item re-selection requires multiple clicks"?

[hotyhuang]

@mcMickJuice yep, just did. Thanks for treating these as "bug", and frequent updates of the library. Although there are some issues, I have to say this is really an awesome and flexible library!

[vic-yoshitom]

hi @mcMickJuice

I found same situation in interactjs. The below URL is the issue about this situation.

• URL

It seems that this bug was fixed in latest interactjs.
But react-calendar-timeline does not require latest interactjs. (The latest version is 1.3.4, react-calendar-timeline requires 1.2.6.)
Would you upgrade interactjs version?

[mcMickJuice]

@vic-yoshitom thanks for the tip!

FIx for this will go out in next release. I moved interactjs to peerDependency and specified 1.3.4 as version.

[mcMickJuice]

Fixed in 0.17.0

[vic-yoshitom]

you are welcome.
it is my pleasure to help you.

[ctrldavid]

Is there a reason you've made interactjs a peer dependency rather than just updating the version you require?

[mcMickJuice]

Nope other than:

• the readme states that interact is a peer dep (even before this change) so I wanted to be consistent with the docs
• I don't see the harm in making it a peer dependency and it might be more appropriate that it is one, given situations like this

Do you disagree? What are reasons not to list it as a peer Dep?

[ctrldavid]

I'm still trying to get my head around what warrants promoting a dependency to a peer dependency myself. Reading: https://nodejs.org/en/blog/npm/peer-dependencies/ seems to indicate that it was introduced for 'plugins' that enhance the functionality of the peers. In which case I'd probably say that react + react dom should be peers, but moment and interactjs would be regular dependencies.

I guess my personal take is that to use react-calendar-timeline I HAVE to be using react/react-dom, but I don't necessarily personally need to use moment or interactjs. As it stands if someone was cleaning up the package.json, saw interactjs and searched the codebase they would probably think it could be removed because it's not used anywhere.

We do use moment, but now there is a risk that moment could release a new major version that breaks react-calendar-timeline. If we update moment then react-calendar-timeline would stop working, so we'd be forced to stay on an out of date version till you release an update yourself. If you left it as a regular dependency then everything would keep working (at the cost of having to bundle 2 versions of moment)

I'm going to do a bit of googling to see if anyone has put out guidelines on how we should be using peer dependencies because it seems a little arbitrary to me still.

[amureki]

Greetings, fellows.

In one of the codebases I realized, that we have an interactjs first-level dependency, and we are never using it in our code, that made me wonder why do we need it. Only after trying to build the bundle, I've got an error

Module not found: Error: Can't resolve 'interactjs' in '.../node_modules/react-calendar-timeline/lib/lib/items'

So, react-calendar-timeline seems to be not working without interactjs. To me, that is a clear sign of it being a normal dependency, not a peer-dependency.

To give you a better grasp of how I see it, I found this article:
https://medium.com/angular-in-depth/npm-peer-dependencies-f843f3ac4e7f

Here are couple points from it:

When you add a package in dependencies, you are saying:

• My code needs this package to run.
• If this package doesn’t already exist in my node_modules directory, then add it automatically.
• Furthermore, add the packages that are listed in the package’s dependencies. These packages are called transitive dependencies.

By adding a package in peerDependencies you are saying:

• My code is compatible with this version of the package.
• If this package already exists in node_modules, do nothing.
• If this package doesn’t already exist in the node_modules directory or it is the wrong version, don’t add it. But, show a warning to the user that it wasn’t found.

So, I'd like to propose (but of course, it is up to maintainers to agree/disagree) to put all dependencies that are _required_ for react-calendar-timeline under dependencies list with a range of tested versions, maybe limiting to not allow next major versions without a manual check.
I am happy to tackle this change, if this proposal will be accepted.

Best,
Rust