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

fix(VCalendar): calender displayed incorrectly when weekdays changed #20378

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

wyhsunflower
Copy link

Description

Fix: When changing from "sunday-saturday" to "mon-sun", calender displayed incorrectly.
fixes:#19999

Markup:

  <v-row>
    <v-col cols="12">
      <v-sheet>
        <v-select
          v-model="weekday"
          :items="weekdays"
          class="ma-2"
          label="weekdays"
          variant="outlined"
          dense
          hide-details
        />
      </v-sheet>
    </v-col>
    <v-col cols="12">
      <v-sheet>
        <v-calendar
          ref="calendar"
          :events="events"
          :weekdays="weekday"
          color="primary"
          type="week"
        />
      </v-sheet>
    </v-col>
  </v-row>
</template>

<script>
  export default {
    data: () => ({
      today: '2019-01-08',
      weekday: [1, 2, 3, 4, 5, 6, 0],
      weekdays: [
        { title: 'Sun - Sat', value: [0, 1, 2, 3, 4, 5, 6] },
        { title: 'Mon - Sun', value: [1, 2, 3, 4, 5, 6, 0] },
        { title: 'Tu - Mo', value: [2, 3, 4, 5, 6, 0, 1] },
        { title: 'Wed - Tu', value: [3, 4, 5, 6, 0, 1, 2] },
        { title: 'Th - Wed', value: [4, 5, 6, 0, 1, 2, 3] },
        { title: 'Fri - Th', value: [5, 6, 0, 1, 2, 3, 4] },
        { title: 'Sat - Fri', value: [6, 0, 1, 2, 3, 4, 5] },
        { title: 'Mon - Fri', value: [1, 2, 3, 4, 5] },
        { title: 'Tu - Fri', value: [2, 3, 4, 5] },
        { title: 'Mon, Wed, Fri', value: [1, 3, 5] },
      ],
      events: [
        {
          title: 'Weekly Meeting',
          start: new Date('2019-01-07 09:00'),
          end: new Date('2019-01-07 10:00'),
        },
        {
          title: `Thomas' Birthday`,
          start: new Date('2019-01-10'),
          end: new Date('2019-01-10'),
          allDay: true,
        },
        {
          title: 'Mash Potatoes',
          start: new Date('2019-01-09 12:30'),
          end: new Date('2019-01-09 15:30'),
        },
      ],
    }),
  }
</script>

@blalan05
Copy link
Member

We cannot change the way the adapter works. It needs to remain compatible with date-io: https://github.com/dmtrKovalenko/date-io/blob/4fe384885f7d99119bb4a63d8b9b5c3031bdf22d/packages/core/IUtils.d.ts#L190

@blalan05 blalan05 self-requested a review August 23, 2024 02:01
@blalan05 blalan05 added the C: VCalendar VCalendar label Aug 23, 2024
@blalan05 blalan05 marked this pull request as draft August 23, 2024 02:02
@wyhsunflower
Copy link
Author

@blalan05 Hi, thanks for review, so you mean, "getWeekArray" already has the firstDayOfWeek, cannot have one more?
how about we put it in one parameter? getWeekArray (date: T, {firstDayOfWeek?: number | string, weekDays?: number[]} ): T[][]

@blalan05
Copy link
Member

Right, it can only have one parameter: a JS Date Object, so no we still can't make it all one object. It will need to be handled outside of the adapter. Probably in the Calendar Composable, because VDatePicker would need the same functionality.

@wyhsunflower
Copy link
Author

@blalan05 Ok, I can try to fix it in Calendar Composable, but why "firstDayOfWeek" is acceptable?

@blalan05
Copy link
Member

@blalan05 Ok, I can try to fix it in Calendar Composable, but why "firstDayOfWeek" is acceptable?

I believe this is a temporary bug. It needs fixed.

@kayhern kayhern force-pushed the fix-issue-19999-VCalendar-weekdays-not-changing-properly branch from 13b9eac to 928727f Compare August 27, 2024 17:51
@wyhsunflower
Copy link
Author

@blalan05 Hi , I re-commit my change, fixing the issue in composables/calendar.ts.

@wyhsunflower wyhsunflower marked this pull request as ready for review August 28, 2024 06:04
@blalan05
Copy link
Member

Its in the right place, however, you are using the bug in this fix, which I don't think is helpful in the long run. I think this is currently difficult to solve because: each locale has a default firstDayOfWeek, we have the firstDayOfWeek prop and then weekDays array prop. I, initially, wrote it so that weekDays prop does not control order, only visibility. If we have/keep the firstDayOfWeek prop, then I think we should also not allow weekDays to control order.

@wyhsunflower
Copy link
Author

@blalan05 I didnt got you point. It make sense if we use the order of the weekDays for calendar, it has default value in Vuetify API, but "first-day-of-week" not. We can modify the API page for "first-day-of-week" to understand that "weekDays" and "first-day-of-week" are mutually exclusive. VCalendar only accept one of them.

@KaelWD KaelWD force-pushed the master branch 3 times, most recently from 4c970f9 to 6a3285f Compare September 3, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VCalendar VCalendar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants