Skip to content

✨ Feat(web): Add move to next/prev week/month shortcuts for someday event#559

Merged
that-one-arab merged 1 commit intomainfrom
feature/526-add-shortcuts-to-migrate-someday-event-buttons
Jun 24, 2025
Merged

✨ Feat(web): Add move to next/prev week/month shortcuts for someday event#559
that-one-arab merged 1 commit intomainfrom
feature/526-add-shortcuts-to-migrate-someday-event-buttons

Conversation

@that-one-arab
Copy link
Contributor

Description

Closes #526

@that-one-arab that-one-arab merged commit 3f1a910 into main Jun 24, 2025
4 checks passed
@that-one-arab that-one-arab deleted the feature/526-add-shortcuts-to-migrate-someday-event-buttons branch June 24, 2025 18:42
event: Schema_Event,
): Schema_Event => {
const reference =
to.direction === "current" ? dayjs(new Date()) : dayjs(event.startDate);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why use dayjs(new Date()) instead of dayjs()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhhhh no particular reason, I guess I was used to it but now after reading your comment I realized that the latter is the better practice, thanks for the headsup

};
};

export const setEventStartEndDates = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add some unit tests for this, as I'm already seeing some bugs with its implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sure, but can you share what bugs did you stumble upon?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

};
};

export const setEventStartEndDates = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename this so it doesn't start with set.... Since this isn't changing any React state, I think we should stay away from set... as the first part of a function to avoid confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes makes sense, thanks for pointing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add shortcuts to migrate someday event buttons

2 participants