WickyNilliams / cally

Small, feature-rich calendar components
https://wicky.nillia.ms/cally/
MIT License
1.06k stars 12 forks source link

Calendar Dynamic Rendering Issue on Multi-Month Display #43

Closed Cata1989 closed 1 month ago

Cata1989 commented 1 month ago

I would like to open an issue regarding an observed issue within the library. The problem arises when there are multiple months involved, particularly when the HTML rendering becomes dynamic based on the number of months or the type of calendar (range or multi). The issue manifests when, for instance, two months are displayed, and a user selects dates from both. Upon clicking "next" to proceed to the next month and selecting a date, the calendar automatically re-renders, reverting to the initial state. This behavior is undesirable. Interestingly, this issue does not occur when static HTML is utilized, as demonstrated below:

<calendar-multi months={3}>
   <calendar-month></calendar-month>
   <calendar-month offset={1}></calendar-month>
   <calendar-month offset={2}></calendar-month>
</calendar-multi>

https://github.com/WickyNilliams/cally/assets/98550729/62052554-071e-4835-b43e-12ba4fa3fb53

Please review and provide any necessary corrections or suggestions.

Thank you!

endv-bogdanb commented 1 month ago

We investigate further and the issue occurs when focused-date is changed dynamically and assigned to current value.

You can paste this in index.html file and open it in browser to reproduce the issue in the video.

<wrapper-calendar></wrapper-calendar>

<script type="module">
    import { html, LitElement } from 'https://cdn.jsdelivr.net/gh/lit/dist@2.4.0/core/lit-core.min.js';
    import "https://unpkg.com/cally"

    export class WrapperCalendar extends LitElement {

        static get properties() {
            return { value: { type: String } }
        }

        onChange = (event) => this.value = event.target.value

        render() {
            return html`
            <calendar-multi 
                value=${this.value} 
                focused-date=${this.value}
                @change=${this.onChange}>
                <calendar-month></calendar-month>
            </calendar-multi>
            `;
        }
    }

    customElements.define('wrapper-calendar', WrapperCalendar);
</script>
WickyNilliams commented 1 month ago

Focused date should only ever be a string containing a single ISO date. If you are working with multi date, then you should pick one of the dates to set as focused on value change. This may be the earliest date, the latest date, or any other. Up to you, depending on your needs and use case

WickyNilliams commented 1 month ago

In case it's not clear, focused date is the date which a user can tab to. There is only ever one date with tabindex="0", all others have a negative tabindex. You can think of it like caret or cursor.

If the focused date is set to a date within the current "page" (however many months are shown) then we just update which button has zero tabindex.

If the focused date is set to outside the current page, then the page is moved forward or backward so that the the focused date is visible, and tabindex is set to zero for that date.

Hopefully this clarifies why you must have a single value not multiple