fomantic / Fomantic-UI

Fomantic-UI is the official community fork of Semantic-UI
https://fomantic-ui.com
MIT License
3.52k stars 329 forks source link

[calendar] incorrect day shown when startMode changed from "month" to "hour" #3031

Open daveharris opened 3 months ago

daveharris commented 3 months ago

Bug Report

When a Calendar is initialized from <input value="..."> with startMode: "month" and , and then initialised again with startMode: "hour", the date of the calendar is the 1st of the month. When the calendar is dismissed and re-opened, the date is correctly set.

The calendar is initialized twice because the first time is done across all calendars on the site (i.e. $('.ui.calendar').calendar({})), and then some specific setup of a particular calendar (i.e. $('.calendar.start').calendar({...})) based on the use-case of the page.

Steps to reproduce

  1. Have an <input> HTML element with a value attribute (to set the initial date)
  2. Initialize the calendar with startMode: "month"
  3. Re-initialize the calendar with startMode: "hour"
  4. Click the input to show the calendar. Notice that the day is the 1st of the month. I assume that this is because the startMode was month
  5. Dismiss the calendar by clicking elsewhere. Click the input to show the calendar again. Notice that the date is now correct (from the value attribute)

Expected result

The date is the same date as the value attribute, in this case "April 6"

Actual result

The date is the 1st of the month, in this case "April 1"

Testcase

https://jsfiddle.net/978rct0k/7/

Screenshot

Screenshot 2024-04-08 at 10 35 02 AM

Version

2.9.3

daveharris commented 3 months ago

I have tried working around this via:

However, none of them have worked

daveharris commented 3 months ago

Upon further investigation I found that this was also triggered from code like:

$('.calendar.start').calendar({
  startMode: $('.calendar.start').calendar('get date') ? 'hour' : 'month'
})

I was doing this because if the calendar had a date selected, I wanted to zoom straight to 'hour' mode so they can move it forwards or backwards by an hour easily. This seems a common use case.

I assume that this broke because calling .calendar('get date') internally initializes the Calendar with .calendar({}) and then re-initialises with startMode: 'hour' and thus getting into the same situation as https://jsfiddle.net/978rct0k/7/.

Workaround

In this case I have worked around this issue via:

$('.calendar.start').calendar({
  startMode: $('.calendar.start').find('input').attr('value') ? 'hour' : 'month',
})

and

<input value="2024-04-09T17:00:00+12:00" type="text" name="event[starts_at]">

Where the value attribute is in ISO8601 format so it can be reliably converted to a JS Date via Date.new($('.calendar.start').find('input').attr('value'))

prudho commented 3 months ago

Well this looks like a difficult issue to look at... The more I step into this, the more I wonder how it can be fixed without breaking anything else.

The less code breaker workaround that I found is to do this at the 2nd init:

/* Setup for the start calendar*/
$('.calendar.start').calendar({ startMode: 'hour' }).calendar('set mode', 'hour');

@lubber-de a potential fix would be to add these lines in calendar.js, line 118:

if (settings.startMode !== module.get.mode()) {
    module.set.mode(settings.startMode);
}

But i'm not sure about the isues it could raise.

daveharris commented 3 months ago

Yeah the more I looked into it the weirder it got.

The one thing you could potentially change would be to return undefined from $(...).calendar('get date') if it hasn't been initialized yet. However this would likely be a breaking change and may not be consistent with the other modules.

It also wouldn't help the initial case where it's initialized twice - which seems to be the root cause of the issue

lubber-de commented 3 months ago

Fixed by #3033 See your adjusted jsfiddle here https://jsfiddle.net/lubber/b04zkxf6/

The previously stored settings were not removed on re-instantiation which was causing your described situation

daveharris commented 3 months ago

Wow that's brilliant, thanks for such a fast fix @lubber-de

Out of interest, does it internally call destroy upon (re-)initialisation as I notice that we don't need to explicitly call .calendar('destroy').calendar({...})?

lubber-de commented 3 months ago

Yes, all FUI js modules call destroy upon re-instantiation automatically.