-
Notifications
You must be signed in to change notification settings - Fork 1
Om corpset #392
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
base: main
Are you sure you want to change the base?
Om corpset #392
Conversation
CodeyBoi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes need to be made, and bun format && bun lint:fix needs to be ran, but this looks like a very good start!
| label: i.instrument.name, | ||
| })); | ||
|
|
||
| const isTrivselOmbud = corps?.roles.some(role => role.name === "Trivselombud"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to keep the code in English. Although in this case it might be acceptable as we are looking for a role called "Trivselombud".
| language String @default("sv") | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| contactURL String? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not 100% convinced adding contactURL to Corps is a good idea. I would prefer adding a new model Position which defines a name, contactUrl and maybe links to some of the permission roles.
E.g. for the Kassör:
{
name: "Kassör",
nameEn: "Treasurer",
email: "kassor@bleckhornen.org",
contactUrl: null, // Google form for Trivselombud
permissionRoleId: <id>,
}Another solution could be to add email and contactUrl to Role, and create new roles for each board member and commitee chairman. Could be worth adding nameEn/nameEnglish as well to store the English translation.
| }; | ||
|
|
||
|
|
||
| const Positions = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be called InfoPage.
| const styrelseOrder: Dictionary<number> = { | ||
| "Ordförande": 1, | ||
| "ViceOrdförande": 2, | ||
| "Sekreterare": 3, | ||
| "Kassör": 4, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems overcomplicated. Just have boardRoles as an array of strings and collect each corps by checking which corps have that role.
You could also define a new API router endpoint getBoardRoles or getCommiteeLeaders which collects all corps in the relevant positions.
| ) | ||
| ); | ||
|
|
||
| const TrivselCorps = await Promise.all( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be in camelCase.
| <h4>Import</h4> | ||
|
|
||
| </div> | ||
| {lang(`Vi i importen ser till att det finns den finaste ölen och cidern till ett överkomligt pris. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave out the "överkomligt pris" part. We are not a store.
| </ActionIcon> | ||
| </div> | ||
| {lang(`ITK har ansvar för drift av alla Bleckhornens hemsidor, samt vidareutveckling av Blindtarmen. | ||
| Driftansvaret includerar blindtarmen, den publika hemsidan och vår interna wiki.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Blindtarmen" should be capitalized.
| <h2>{lang('Ansvarsposter', 'Positions of responsibility')}</h2> | ||
| <div className='max-w-4xl'> | ||
| <div className='flex flex-col space-y-2 p-4'> | ||
| <h3>{lang('Styrelsen', 'The Board')}</h3> | ||
| {lang(`Styrelsen leder Bleckhornen under ett verksamhetsår. Tillsammans med dirigenter, | ||
| balettledare, utskottsmedlemmar och andra förtroendevalda ansvarar styrelsen för planering | ||
| av repor, spelningar, sittningar, konserter, resor och andra aktiviteter. Detta görs med stöd och | ||
| samarbete med hela corpset`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we should decide which terms should be capitalized or not, so the use remains consistent. Right now it is a bit mixed.
| 'Samply', | ||
| "Samply", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a more descriptive title.
| }, | ||
| { label: lang('Sånger', 'Songs'), href: '/songs', icon: <IconMusic /> }, | ||
| { label: lang('Länkar', 'Links'), href: '/links', icon: <IconLink /> }, | ||
| { label: lang('Om Bleckhornen', 'About Bleckhornen'), href: '/info', icon: <IconInfoHexagon /> }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use IconInfoCircle instead. More thematic with Bleckhornen.
fixes #340