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

[INT-93] Different colours for event types #189

Merged

Conversation

ben260
Copy link
Contributor

@ben260 ben260 commented Oct 23, 2024

Added new workshop type and permissions for it.
Added new component for colour types key on the calendar page.
Added workshop to event type selector when creating new event.

Testing:

Screenshot 2024-10-23 173342
Screenshot 2024-10-23 174327

@archessmn archessmn changed the title Int 93 can we have different colours for different event types [INT-93] Different colours for event types Oct 23, 2024
@ben260 ben260 requested a review from archessmn October 23, 2024 17:02
@@ -381,7 +389,7 @@ export default function YSTVCalendar() {
);
}

type EventType = "show" | "meeting" | "social" | "other";
//type EventType = "show" | "meeting" | "social" | "other";
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this if it isn't used?

@@ -52,7 +52,7 @@ export default async function NewEventPage() {
);
if (permittedEventTypes.length === 0) {
throw new Forbidden([
"Calendar.Admin or Calendar.{Show,Meeting,Social}.{Creator,Admin}" as any,
"Calendar.Admin or Calendar.{Show,Meeting,Social,Workshop}.{Creator,Admin}" as any,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Calendar.Admin or Calendar.{Show,Meeting,Social,Workshop}.{Creator,Admin}" as any,
"Calendar.Admin or Calendar.{Show,Meeting,Workshop,Social}.{Creator,Admin}" as any,

Very much a nitpick but just to have these in the same order as elsewhere 😂

@@ -30,6 +37,9 @@ export function adminEventTypes(userPermissions: Permission[]): EventType[] {
if (userPermissions.includes("Calendar.Public.Admin")) {
permittedEventTypes.push("public");
}
if (userPermissions.includes("Calendar.Workshop.Admin")) {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick again, reorder this to the same order as EventTypes

@@ -54,6 +64,9 @@ export function creatableEventTypes(
if (userPermissions.includes("Calendar.Public.Creator")) {
base.push("public");
}
if (userPermissions.includes("Calendar.Workshop.Creator")) {
Copy link
Member

Choose a reason for hiding this comment

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

^ Reorder

lib/auth/permissions.ts Outdated Show resolved Hide resolved
@jenkins-ystv
Copy link

jenkins-ystv bot commented Oct 24, 2024

Attempted to deploy a preview of this PR, but ran into an error. See the Jenkins output for details.

@ben260 ben260 merged commit 792e8da into main Oct 24, 2024
3 checks passed
@archessmn archessmn deleted the int-93-can-we-have-different-colours-for-different-event-types branch October 24, 2024 20:58
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.

2 participants