Draft: Implemented Calendar List View

[tylersocholotuik]

What issue is this referencing?

#394 specifically, this comment

Here is the first iteration of the idea @dgmouris had for the calendar list view. The events should be grouped by month, but there are unfortunately only events in November at the moment so I am not certain this works.

Let me know your thoughts on anything that could be improved!

Do these code changes work locally and have you tested that they fix the issue yourself?

• yes!

Does the following command run without warnings or errors?

• yes! npm run pr-checks

Have you taken a look at our contributing guidelines?

• yes!

My node version matches the one suggested when running nvm use?

• yes!

[tylersocholotuik]

Update: I fixed the height of the cards based on feedback from @johnjovero98, and I set the default view to list on mobile (<768px).

[MandyMeindersma]

LOVE this! I am going to let another person look this over too.

I am a BE dev by day so my FE is a little rusty

[arashsheyda]

the card width on mobile view is not quite responsive I think. also if we use computed for isMobile it would be better (I think we can even combine selectedView with defaultView using a computed)

[tylersocholotuik]

Ah ok I will try to fix that. I think the calendar view has the same issue on mobile as well.

[arashsheyda]

@tylersocholotuik thanks! (it's not a big deal though, if you ask me. we can merge it for now and add a fix later. looks great! thanks)

[tylersocholotuik]

@tylersocholotuik thanks! (it's not a big deal though, if you ask me. we can merge it for now and add a fix later. looks great! thanks)

Ok sounds good! If I have some time later today I will try to tackle the responsive issue. Changing isMobile and selected view to computed caused an issue, however.

const isMobile = computed(() => useMediaQuery('(max-width: 768px)').value)
const selectedView = computed(() => isMobile.value ? 'list' : 'calendar')

This will now change the view automatically based on the window size, and clicking on the tabs no longer works. Is there something I am missing?

My original idea was that the default view is set only when the page loads based on the window size, then the user will have control with the tabs after that. If we want both (automatic switching + user control with tabs), I'm not sure how to get the tabs to work now.

[arashsheyda]

selectedView should stay ref in this case.

[tylersocholotuik]

selectedView should stay ref in this case.

ok, so keep isMobile as computed, but change the others back to the way it was before?

const isMobile = computed(() => useMediaQuery('(max-width: 768px)').value)
const defaultView = isMobile.value ? 'list' : 'calendar'
const selectedView = ref(defaultView)

I could put the expression for defaultView in the ref if you prefer fewer lines.

[arashsheyda]

@tylersocholotuik my bad, I didn't notice useMediaQuery is from @vueuse and it is a computed already so no need for that. but how about something lik this:

const isMobile = useMediaQuery('(max-width: 768px)')
const calendarView = useCookie('calendarView', { default: () => 'calendar' })
const selectedView = computed({
  get: () => isMobile.value ? 'list' : calendarView.value,
  set: value => calendarView.value = value,
})

this way we save the users choice (in cookies) also do you think we should hide the tabs in mobile view since it wouldn't change the view anyway?
sorry again for the confusion about useMediaQuery

[tylersocholotuik]

@arashsheyda thanks for the help with the view selection, I have not learned about cookies yet. I made that change, plus hid the tabs on mobile view and changed the border colors.

[tylersocholotuik]

I realized I may have commited too hastily. I had the thought that maybe it isn't a good idea to prevent the option of using the calendar view on mobile? Or maybe I can change the media query to a smaller size where the calendar view really doesn't look great.

[dgmouris]

@arashsheyda thanks for the thorough review and @tylersocholotuik for the work.

On mobile, the calendar view might need some work, and it's a bit difficult to use (I think it'll be some vue-cal config options).

I think the cookie is great but as long as we have the default as the list view that's awesome.

I'm a fan of this implementation

const isMobile = useMediaQuery('(max-width: 768px)')
const calendarView = useCookie('calendarView', { default: () => 'calendar' })
const selectedView = computed({
  get: () => isMobile.value ? 'list' : calendarView.value,
  set: value => calendarView.value = value,
})

[dgmouris]

@tylersocholotuik sometimes the events are going to have a bit of a longer description because of markdown from.

This technique I wouldn't use all the time but I think this will solve our problem and look good.

Suggested change

	<div class="p-4 event-description">
	<div
	class="p-4 event-description"
	:class="{ 'max-w-sm': isMobile }"
	>

The mobile view on pixel 7:

The desktop view:

@arashsheyda @tylersocholotuik let me know what you think.

I'd be ready to merge this once this implemented so that it doesn't break on mobile formats.

[dgmouris]

Awesome I like the use of using a computed value to group the events. I personally might change the acc on line 26 to something like monthEvents but not a show stopper whatsoever.

[dgmouris]

Once the above is fixed I'm good with merging this in:)

Great job @tylersocholotuik and thanks @arashsheyda, @MandyMeindersma for reviewing this too:)

[tylersocholotuik]

@dgmouris Thanks a lot for all the suggestions! I made all the changes you suggested and I also made the time and date on the cards move underneath the title header on mobile view to help with the width overflow issue. I also decided to do the media query at 700px rather than 768px because I think people would like to use the calendar view on tablets. Let me know if you agree with that.

Here is the new mobile view