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.

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

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

Open
daveharris opened this issue Apr 8, 2024 · 7 comments
Labels
tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build type/bug Any issue which is a bug or PR which fixes a bug
Milestone

Comments

@daveharris
Copy link

daveharris commented Apr 8, 2024

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 daveharris added state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/bug Any issue which is a bug or PR which fixes a bug labels Apr 8, 2024
@daveharris
Copy link
Author

I have tried working around this via:

  • .calendar('set mode', 'hour')
  • .calendar('refresh')
  • .calendar('destroy')

However, none of them have worked

@daveharris
Copy link
Author

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
Copy link
Contributor

prudho commented Apr 10, 2024

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
Copy link
Author

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
Copy link
Member

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

@lubber-de lubber-de added state/has-pr An issue which has a related PR open and removed state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged labels Apr 16, 2024
@lubber-de lubber-de added this to the 2.9.4 milestone Apr 16, 2024
@daveharris
Copy link
Author

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
Copy link
Member

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

@lubber-de lubber-de added tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build and removed state/has-pr An issue which has a related PR open labels Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

No branches or pull requests

3 participants