-
Notifications
You must be signed in to change notification settings - Fork 7
feat: react attendance pages #239
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?
Conversation
0368920 to
f4244e9
Compare
c877be0 to
256d8d3
Compare
| } | ||
|
|
||
| getActivist(name: string): { | ||
| exists: boolean |
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.
returning null would be more idiomatic, and prevent accidental usage of other fields under the false assumption that the activist must exist if the fields are accessible, and would simplify this implementation (no need optional chaining operator).
| } | ||
| } | ||
|
|
||
| function nameFilter(text: string, input: string): boolean { |
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.
can you add a doc comment for this function? not obvious what it does after staring at it for a little bit
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 this file be moved to event/new/page.tsx, to make room for a future event/page.tsx that lists all the events?
and should /event be renamed to /events for consistency with /users? cursory google search says plural is preferred
sorry just noticed this now
No description provided.