-
Notifications
You must be signed in to change notification settings - Fork 48
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
Nishita_develop_frontend_for_event_participation_analysis #3052
base: development
Are you sure you want to change the base?
Nishita_develop_frontend_for_event_participation_analysis #3052
Conversation
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
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.
Good progress on implementing the event participation landing page. Some suggestions for improvement:
Component Structure:
Missing error handling for date parsing in getDateRange function
Select elements need associated labels for accessibility
Consider memoizing filteredEvents to improve performance
Suggested Improvements:
Add PropTypes/TypeScript for type safety
Add aria-labels for color-based indicators (tracking-rate-green/red)
Document expected mockData format
Consider separating filter logic into a custom hook
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’ve noticed two small issues:
- There’s a small alignment issue in the list view.
- The time filter doesn't seem to work quite right. When selecting "This Week" or "This Month," any events with AM times don’t appear. This seems to be caused by the code that splits the time in mockData:
const eventDate = new Date(event.eventTime.split(' pm ')[1])
;.
.case-list-item { | ||
display: flex; | ||
justify-content: space-between; | ||
align-items: center; |
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.
The items don't seem to be perfectly aligned in the list view. Would it be helpful to consider using flex-basis
, flex-grow
, and flex-shrink
instead of justify-content
?
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.
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.
Hi @Eveye2023, I have fixed the alignment issue. Please feel free to review the updated changes.
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.
Hi @Eveye2023 , I have fixed the alignment issue.
For now, the goal was to establish the foundational structure for displaying the data. I’ll ensure that once the backend is set up, The filter functionality is incorporated based on how the data is being stored. Thanks for your review and insights!
Hi @Sheehan20, For now, the goal was to establish the foundational structure for displaying the data, and I’ll ensure these suggestions are incorporated during the integration phase. Thanks again for your review and insights! |
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.
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.
The changes that are aforementioned are functioning as expected.
Description
Implemented the frontend of the Landing Page for Event Popularity Analytics where the user can see the Drop-off and No-show event analysis.
Related PRS (if any):
No related PRs
…
Main changes explained:
…
How to test:
npm install
and...
to run this PR locallyScreenshots or videos of changes:
Recording.2025-01-18.003537.mp4
Note:
The mockData is being used to verify the UI as the backend is not yet implemented.