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

Polish Calendar UI (month view) #424

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Conversation

johnjovero98
Copy link
Contributor

@johnjovero98 johnjovero98 commented Oct 19, 2024

What issue is this referencing?

#394

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!

Here is a demo gif of the calendar redesign.

Note: I used pure CSS to make the changes instead of tailwind classes. I'm not sure if that will cause any performance issues. Please let me know if I need to refactor it :>

month-view-demo

Copy link
Collaborator

@dgmouris dgmouris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but we should rebase and remove the compabilityDate if not needed.

@@ -35,7 +33,7 @@ const onEventClick = (event: any, e: any) => {
:events-on-month-view="true"
:twelve-hour="true"
:events="group.items"
:start-week-on-sunday="false"
:start-week-on-sunday="true"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might have to rebase this branch because due to another PR that put this in.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically it won't overwrite because it's the same line.

nuxt.config.ts Outdated
@@ -82,6 +81,8 @@ export default defineNuxtConfig({
componentIslands: true,
},

compatibilityDate: '2024-10-18',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this is for, was this needed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is from Nuxt. we don't necessarily need to include it in the PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good @arashsheyda. I think it's good to merge once this is removed! Let me know what you think.

@johnjovero98 could you remove this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed line 84 and got this branch up to date :>

Screenshot 2024-10-21 at 3 21 09 PM

@dgmouris
Copy link
Collaborator

Thanks @johnjovero98 this is great I'm merging this.

@dgmouris dgmouris merged commit c219952 into devedmonton:main Oct 22, 2024
4 checks passed
@MandyMeindersma
Copy link
Contributor

@allcontributors add @johnjovero98 for code

Copy link
Contributor

@MandyMeindersma

I've put up a pull request to add @johnjovero98! 🎉

@dgmouris dgmouris added the hacktoberfest-accepted PRs that are for the month of October and will count towards hacktoberfest PRs label Oct 25, 2024
@johnjovero98 johnjovero98 deleted the calendar-394 branch November 12, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted PRs that are for the month of October and will count towards hacktoberfest PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants