React9k / react-timeline-9000

React Timeline
http://react-timeline-9000.s3-website-ap-southeast-2.amazonaws.com/
MIT License
288 stars 36 forks source link

[rt10000] Not using moment.js for dates (to be able to store the state in Redux) #221

Closed cristian-spiescu closed 2 years ago

cristian-spiescu commented 3 years ago

Hi there,

Currently the the model elements (items, layers) NEED to have start/end dates as "moment.js" objects.

                    let item: FlightItem = {
                        row: i,
                        start: moment(startDate),
                        end: moment(endDate),
                        key: flight.id,
                        data: flight
                    };

It would be interesting to also accept "primitive" data types. E.g. number = millis. Advantage = this way we might store the entire Gantt state in a Redux state. Which doesn't accept non-serializable types (i.e. the case of "moment.js").

We'd like to take a look to see if this is difficult to implement. Any objections and/or hints?

Regards, Cristian.

lilfolr commented 3 years ago

No objections. I think this could be done in line with the suggestions in #214. Ideally we'll use a smaller moment replacement for calculations, and try pass around epoch timestamps wherever possible

cristian-spiescu commented 2 years ago

Hi!

Please take a look at the associated PR for this discussion, #241. We added also this doc. I recommend you to take a look before reviewing.

Practically now all manipulation (read/write) of dates pass through a family of functions, such as getStartDate(), getEndDate(),getStartFromItem(item), etc. The default implementation of these functions takes into account the property useMoment. If true => we are in the initial mode, where the user supplies date in "moment" format. If false => we are in the new mode, where the user supplies dates as timestamps = millis. I remind that this is convenient for people using Redux, because Redux doesn't allow having non-primitive types (such as moment) in the state.

Note: an user may extend Timeline and override these functions for other purposes as well. E.g. to customize how start/end is retrieved for an item, if that item doesn't use the default props, or if there is a small calculation involved. For example, we work w/ flights, which don't have start / end. They only have one time. And the other one is calculated by adding a constant to it.

cristian-spiescu commented 2 years ago

Hi!

We fixed the issue and answered your review questions. Could you take a new look and try to merge the PR?

Thanks!

cristian-spiescu commented 2 years ago

Hi. Do you think you'll have a bit of time to take a look at the associated PR? It's an important update, and we'd like it to have it in the mainstream to avoid conflicts & co. Thanks.

lilfolr commented 2 years ago

Merged. Thanks!