Add tags and changed display of speakers #218
Conversation
Reviewer's Guide by SourceryThis pull request implements several changes to the schedule view of the web application, including the addition of tags to sessions, modifications to the display of speakers, and updates to the schedule components. The changes primarily focus on enhancing the user interface and improving the organization of schedule-related information. File-Level Changes
Tips
|
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @odkhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Ensure that all functionality from the previous GridSchedule implementation is preserved in the new custom version.
- Consider abstracting some of the component-specific styles into separate CSS files for better separation of concerns.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -0,0 +1,437 @@ | |||
| <template lang="pug"> | |||
There was a problem hiding this comment.
suggestion: Consider breaking down GridSchedule component
The GridSchedule component is quite large and complex. Consider breaking it down into smaller, more manageable sub-components to improve readability and maintainability.
<template lang="pug">
.c-grid-schedule
GridHeader
GridBody
GridFooter
| hasAmPm () { | ||
| return moment.localeData().longDateFormat('LT').endsWith(' A') | ||
| }, | ||
| timeslices () { |
There was a problem hiding this comment.
suggestion (performance): Optimize timeslices computation for performance
The timeslices computation seems complex and might be performance-intensive. Consider memoizing this computation or optimizing the algorithm to improve rendering performance, especially for large schedules.
timeslices: _.memoize(function () {
const minimumSliceMins = 30
const slices = []
// ... rest of the function implementation
return slices
}),
| } | ||
| }, | ||
| methods: { | ||
| getContrastColor(bgColor) { |
There was a problem hiding this comment.
suggestion: Move color contrast calculation to a utility function
The getContrastColor function could be moved to a separate utility file. This would make it reusable across components and easier to test independently.
import { getContrastColor } from '@/utils/colorUtils';
// ...
methods: {
// Other methods...
},
computed: {
contrastColor() {
return getContrastColor(this.bgColor);
},
|
Please only fix docker build as well. |
This PR closes/references issue #209 . It does so by:
How has this been tested?
Checklist
Summary by Sourcery
Introduce tags to session and schedule pages and update the display of speakers. Refactor schedule components for better modularity and maintainability.
New Features:
Enhancements: