Fix/list all events in previous events view #302
Conversation
- Implemented dynamic pagination for 'Tidligere' (past events) with ellipsis for long page lists to enhance user navigation. - Adjusted component structure to ensure pagination controls render directly under the events list. - Added conditional rendering and CSS tweaks to handle cases with numerous pages. - Reduced event overview width for better alignment and display consistency across pages. - Included error handling and pagination controls styling adjustments for improved UI feedback on navigation buttons.
♻️ Refactor getPastEventsCount function to use the custom get function instead of fetch for consistency
✅ Add error handling in frontend for getPastEventsCount to handle exceptions gracefully
🔧 Adjust frontend proxy configuration to correctly route API calls to the backend server
| @@ -46,6 +48,11 @@ const EventOverview: React.FC = () => { | |||
| const [pastEvents, setPastEvents] = useState<Event[] | undefined>(); | |||
| const [pastErrorMsg, setPastErrorMsg] = useState<string>(''); | |||
| const { addToast } = useToast(); | |||
| const [skip, setSkip] = useState<number>(0); | |||
| const [limit] = useState<number>(10); | |||
There was a problem hiding this comment.
Since this value is set to 10 and is never changed, it's better to set it to a constant value rather than a useState.
| @@ -55,8 +55,10 @@ export const deleteEvent = (eid: string) => Delete<{}>('event/' + eid); | |||
| export const getUpcomingEvents = (): Promise<Event[]> => | |||
| get<Event[]>('event/upcoming'); | |||
|
|
|||
| export const getPastEvents = (): Promise<Event[]> => | |||
| get<Event[]>('event/past-events'); | |||
| export const getPastEvents = (skip: number, limit: number): Promise<Event[]> => | |||
There was a problem hiding this comment.
It's generally fine to send limit as a dynamic value here to the endpoint. However, because you're only using a constant value of 10 at the moment, could it be better/easier to simply set it to 10 statically in the endpoint? Both solutions are okay in this case
| @@ -132,6 +196,57 @@ const EventOverview: React.FC = () => { | |||
| /> | |||
| </LoadingWrapper> | |||
| )} | |||
| <div | |||
| className="paginationControls" | |||
| style={{ marginBottom: '20px' }}> | |||
There was a problem hiding this comment.
For consistency, you should choose between providing styles in the stylesheet file eventOverview.scss or using style inline like here. Don't use both className and style on the same element, in general.
| i <= Math.min(totalPages - 1, currentPage + 1); | ||
| i++ | ||
| ) { | ||
| pages.push(i); |
There was a problem hiding this comment.
In my opinion, this function would be more readable if the items pushed to the pages list was always of the same data type. So instead of pages.push(i) do pages.push(${i}). (this is nitpicking though)
💎 Changes
Pagination for Past Events: Added dynamic pagination with ellipsis, layout adjustments, and CSS tweaks for better alignment.