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

Fix/list all events in previous events view #302

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/api/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]> =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

get<Event[]>(`event/past-events?skip=${skip}&limit=${limit}`);
export const getPastEventsCount = (): Promise<{ count: number }> =>
get<{ count: number }>('event/past-events/count');

export const getJoinedEvents = (): Promise<Event[]> =>
get<Event[]>('event/joined-events');
Expand Down
37 changes: 36 additions & 1 deletion src/components/pages/events/eventOverview/eventOverview.scss
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
.eventOverview {
display: flex;
flex-direction: column;
min-height: 100vh;
min-height: 50vh;
}

.eventContent {
Expand Down Expand Up @@ -92,6 +92,41 @@
padding-left: 1rem;
}
}
.headerButton {
border: none;
border-radius: 0.5rem;
padding: 0.5rem 1rem;
cursor: pointer;
margin-right: 0.5rem;
}
.paginationControls {
display: flex;
justify-content: center;
align-items: center;
gap: 1rem;
margin-top: 20px;
}
.pageButton {
background: none;
border: 1px solid #ccc;
color: white;
padding: 0.25rem 0.5rem;
cursor: pointer;
min-width: 2rem;
text-align: center;
}

.pageButton.active {
background-color: #2b2c3d;
color: white;
border: 1px solid white;
}
.disabled {
opacity: 0.5;
cursor: not-allowed;
pointer-events: none;
}

@media screen and (max-width: 768px) {
.eventOverviewContainer {
width: 95%;
Expand Down
123 changes: 119 additions & 4 deletions src/components/pages/events/eventOverview/eventOverview.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { useEffect, useState } from 'react';
import './eventOverview.scss';
import Icon from 'components/atoms/icons/icon';
import EventPreview from 'components/molecules/event/eventPreview/EventPreview';
import useUpcomingEvents from 'hooks/useEvents';
import LoadingWrapper from 'components/atoms/loadingWrapper/LoadingWrapper';
Expand All @@ -9,6 +10,7 @@ import { Event } from 'models/apiModels';
import { getJoinedEvents, getPastEvents } from 'api';
import { sortDate } from 'utils/sorting';
import { useToast } from 'hooks/useToast';
import { getPastEventsCount } from 'api';

interface IEventOverview {
events: Event[];
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

const [totalEvents, setTotalEvents] = useState<number>(0);
const totalPages = Math.ceil(totalEvents / limit);
const currentPage = Math.floor(skip / limit) + 1;

const fetchEvents = async () => {
try {
Expand All @@ -61,7 +68,7 @@ const EventOverview: React.FC = () => {
}
try {
/* Get past events */
const past = await getPastEvents();
const past = await getPastEvents(skip, limit);
past.sort((a: Event, b: Event) =>
sortDate(new Date(b.date), new Date(a.date))
);
Expand All @@ -70,6 +77,25 @@ const EventOverview: React.FC = () => {
setPastErrorMsg('En ukjent feil skjedde');
}
};
useEffect(() => {
fetchEvents();
}, [skip]);
useEffect(() => {
const fetchTotalEvents = async () => {
try {
const data = await getPastEventsCount();
setTotalEvents(data.count);
} catch (error) {
console.error('Error fetching total past events:', error);
setTotalEvents(0);
addToast({
title: 'Failed to fetch past events count',
status: 'error',
});
}
};
fetchTotalEvents();
}, []);

useEffect(() => {
if (eventContent === 'my events' && joinedErrorMsg.length) {
Expand All @@ -87,10 +113,48 @@ const EventOverview: React.FC = () => {
}
}, [eventContent, joinedErrorMsg, pastErrorMsg, addToast]);

useEffect(() => {
fetchEvents();
}, []);
const handleNextPage = () => {
if (currentPage < totalPages) {
setSkip((prevSkip) => prevSkip + limit);
}
};

const handlePreviousPage = () => {
if (currentPage > 1) {
setSkip((prevSkip) => Math.max(prevSkip - limit, 0));
}
};

const generatePages = () => {
const pages = [];

// Always show the first page
pages.push(1);

// Show ellipsis if there are several pages between the first and the current page
if (currentPage > 3) {
pages.push('...');
}

// Add pages around the current page
for (
let i = Math.max(2, currentPage - 1);
i <= Math.min(totalPages - 1, currentPage + 1);
i++
) {
pages.push(i);
Copy link
Collaborator

Choose a reason for hiding this comment

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

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)

}

// Add ellipsis and the last page if necessary
if (currentPage < totalPages - 2) {
pages.push('...');
}
if (totalPages > 1) {
pages.push(totalPages);
}

return pages;
};
return (
<div className="eventOverview">
<div className="eventContent">
Expand Down Expand Up @@ -132,6 +196,57 @@ const EventOverview: React.FC = () => {
/>
</LoadingWrapper>
)}
<div
className="paginationControls"
style={{ marginBottom: '20px' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

{eventContent === 'past events' && (
<>
<div
onClick={currentPage === 1 ? undefined : handlePreviousPage}
style={{
opacity: currentPage === 1 ? 0.5 : 1,
cursor: currentPage === 1 ? 'not-allowed' : 'pointer',
pointerEvents: currentPage === 1 ? 'none' : 'auto',
}}>
<Icon type="angle-double-left" size={1.5} />
</div>

{generatePages().map((page, index) =>
page === '...' ? (
<span key={index} className="paginationEllipsis">
...
</span>
) : (
<button
key={index}
className={`pageButton ${
currentPage === page ? 'active' : ''
}`}
onClick={() => {
setSkip((Number(page) - 1) * limit);
fetchEvents();
}}>
{page}
</button>
)
)}

<div
onClick={
currentPage === totalPages ? undefined : handleNextPage
}
style={{
opacity: currentPage === totalPages ? 0.5 : 1,
cursor:
currentPage === totalPages ? 'not-allowed' : 'pointer',
pointerEvents:
currentPage === totalPages ? 'none' : 'auto',
}}>
<Icon type="angle-double-right" size={1.5} />
</div>
</>
)}
</div>
</div>
</div>
</div>
Expand Down
Loading