Feature/events dashboard#58
Conversation
RLee64
left a comment
RLee64
left a comment
There was a problem hiding this comment.
Nice work! Just got some comments to address (mainly to do with synchronising with changes elsewhere).
| export const DEFAULT_USER_ACTION = "SIGN UP"; | ||
| export const DEFAULT_ADMIN_ACTION = "EDIT EVENT"; | ||
|
|
||
| interface EventProps { |
There was a problem hiding this comment.
Have a look at #56 to see the new event data structure. Note that pricing isn't needed atm.
There was a problem hiding this comment.
Just an extra footnote, no need to integrate with backend atm, just good to have some changes that makes the integration a bit smoother.
| description: string; | ||
| memberPrice?: string; | ||
| nonMemberPrice?: string; | ||
| role?: "admin" | "user"; |
There was a problem hiding this comment.
Role can now be retrieved using changes from #44
| }) => { | ||
| const navigate = useNavigate(); | ||
|
|
||
| // Format date: e.g. "2nd April - 6PM" |
There was a problem hiding this comment.
The only relevant part of an event's datetime field field is the date, this means you should ignore any time portion.
| <div className="event-image-container"> | ||
| <ImageBlock | ||
| pageKey={imageUrl || DEFAULT_IMAGE} | ||
| role="user" |
There was a problem hiding this comment.
When merging main you'll notice the role="user" is no a valid field for the component
| <hr className="event-divider" /> | ||
|
|
||
| <button className="rsvp-button" onClick={handleActionClick}> | ||
| {role === "admin" ? DEFAULT_ADMIN_ACTION : DEFAULT_USER_ACTION} |
There was a problem hiding this comment.
Regardless if a user is admin or a user, they should still be taken to the event page, so no need to display user-dependent text.
|
|
||
| <hr className="event-divider" /> | ||
|
|
||
| <button className="rsvp-button" onClick={handleActionClick}> |
There was a problem hiding this comment.
RSVP isn't the best name for this css class, it's just a redirect button
|
|
||
| const Events = () => { | ||
| return <div>Events page - not yet implemented</div>; | ||
| const isAdmin = true; // Hardcoded admin for now |
There was a problem hiding this comment.
No need to hardcode admin anymore
| import eventsData from "../placeholders/events.json"; | ||
| import { Link } from "react-router-dom"; | ||
|
|
||
| const PAGE_TITLE_1 = "Upcoming Events"; |
There was a problem hiding this comment.
nit: no need to declare these headings as constants, they aren't magic numbers/strings so slapping them in is okay 👍
| <div className="event-dashboard"> | ||
| {isAdmin && ( | ||
| <Link | ||
| to="/Events" //add editing at some point |
There was a problem hiding this comment.
the //add editing comment isn't needed, editing occurs on the event page itself, not on the events dashboard
2 participants
events dashboard ordered by time, adding and editing events not implemented yet.