Add Events page from Google Calendar#632
Conversation
|
@fishman: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for project-hami ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ICS calendar ingestion through a Docusaurus plugin, publishes expanded events as global data, and introduces a localized two-week events page with filtering, navigation, agenda rendering, responsive styling, and an Events navbar link. ChangesEvents calendar
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CalendarSource
participant EventsPlugin
participant GlobalData
participant EventsPage
CalendarSource-->>EventsPlugin: provide ICS feed
EventsPlugin->>EventsPlugin: parse and expand VEVENT entries
EventsPlugin->>GlobalData: publish sorted events
EventsPage->>GlobalData: read events
EventsPage-->>EventsPage: filter and group by local day
EventsPage-->>Visitor: render calendar and agenda
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds an Events feature to the HAMi Docusaurus site by ingesting a public Google Calendar ICS feed at build time and rendering a new /events page with a two-week grid, agenda list, and category filtering.
Changes:
- Introduces a custom Docusaurus plugin (
plugin-events) to fetch/expand ICS events and expose them via global data. - Adds a new Events page (
/events) with a two-week grid + agenda view, plus styling. - Wires the plugin and navbar route into
docusaurus.config.js, and addsnode-icalas a dependency.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/plugins/events/index.js | New Docusaurus plugin to fetch and expand calendar events from ICS sources into global data. |
| src/pages/events.js | New Events page consuming plugin global data and rendering a 2-week grid + agenda + filters. |
| src/pages/events.module.css | Styling for the Events page layout, grid cards, agenda list, and responsive behavior. |
| docusaurus.config.js | Registers the events plugin with a Google Calendar ICS source and adds “Events” to the navbar. |
| package.json | Adds node-ical dependency required by the events plugin. |
| package-lock.json | Locks node-ical (and transitive deps) into the dependency graph. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (5)
src/pages/events.js:10
dateKey()usestoISOString()(UTC) to build the YYYY-MM-DD key, but the rest of the page logic (startOfDay,sameDay) works in local time. In non-UTC timezones this can shift the key by a day and mis-bucket events.
const dateKey = (d) => d.toISOString().slice(0, 10);
src/pages/events.js:146
- The previous-period nav button only renders an arrow glyph;
titleisn’t a reliable accessible name for screen readers. Add anaria-labelso the control is announced clearly.
<button
className={styles.navArrow}
onClick={() => setWeekOffset((o) => o - 1)}
disabled={weekOffset <= 0}
title={isZh ? "前两周" : "Previous two weeks"}
src/plugins/events/index.js:1
- This new file isn’t formatted consistently with the repo’s Prettier config (
singleQuote: false), which will causenpm run format:checkto fail. Reformat the file (or runnpm run format) before merging.
import ical from 'node-ical';
src/pages/events.js:160
- The next-period nav button only renders an arrow glyph;
titleisn’t a reliable accessible name for screen readers. Add anaria-labelso the control is announced clearly.
<button
className={styles.navArrow}
onClick={() => setWeekOffset((o) => o + 1)}
title={isZh ? "后两周" : "Next two weeks"}
package.json:45
node-ical@^0.27.1currently resolves to a release that requires Node >=22 (per the lockfile), but this repo’s CI/Netlify builds run Node 20. This dependency range will break installs/builds on the current infrastructure.
"node-ical": "^0.27.1",
hami-robot
Bot
added
dco-signoff: yes
dco-signoff: no
and removed
dco-signoff: no
dco-signoff: yes
labels
The grid derives from new Date() at render time, so the build-time server markup could differ from the client's first render and break hydration. Gate the dynamic calendar section behind useIsBrowser, keeping the hero and CTA server-rendered. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
src/pages/events.js:119
- The category filter buttons behave like a toggle group, but they don’t expose state to assistive tech. Add
aria-pressedto communicate which filter is active.
<button
className={`${styles.filterPill} ${activeCategory === null ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(null)}
>
{isZh ? "全部" : "All"}
src/pages/events.js:126
- The per-category filter buttons should also expose their pressed state (
aria-pressed) so screen readers can tell which category is active.
<button
key={cat}
className={`${styles.filterPill} ${activeCategory === cat ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(activeCategory === cat ? null : cat)}
>
src/pages/events.js:195
-
key={${ev.start}-${ev.summary}}can collide (same title + start time), causing React to reuse DOM nodes incorrectly and log key warnings. Include more fields (end/location/categories) to make the key more unique, or pass through a UID from the ICS source.
<div key={`${ev.start}-${ev.summary}`} className={styles.agendaEvent}>
src/plugins/events/index.js:54
- The catch block logs only
err.message, which can beundefinedfor non-Errorthrows and drops useful stack/context for debugging build failures. Log the full error object instead.
} catch (err) {
console.warn(
`plugin-events: failed to parse "${src.name}" (${src.icsUrl}), skipping:`,
err.message,
);
}
toLocaleDateString output is not guaranteed stable across ICU builds, so derive the local YYYY-MM-DD key from date fields directly. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
Without it the Chinese site shows the English label. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
|
/lgtm |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/pages/events.js:120
- The category filter pills act like toggle buttons, but they don’t expose their pressed state to assistive tech. Add
aria-pressed(andtype="button"to avoid implicit submit behavior if this gets embedded in a form later) for both the “All” button and per-category buttons.
<button
className={`${styles.filterPill} ${activeCategory === null ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(null)}
>
{isZh ? "全部" : "All"}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/events.js (1)
114-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
aria-pressedto category filter toggle buttons.The "All" and category pills visually toggle selected state via CSS class but don't expose it to assistive tech. Since these buttons toggle their own on/off state (not a menu trigger),
aria-pressedis the correct attribute here.♻️ Proposed fix
<button className={`${styles.filterPill} ${activeCategory === null ? styles.filterPillActive : ""}`} onClick={() => setActiveCategory(null)} + aria-pressed={activeCategory === null} > {isZh ? "全部" : "All"} </button> {categories.map((cat) => ( <button key={cat} className={`${styles.filterPill} ${activeCategory === cat ? styles.filterPillActive : ""}`} onClick={() => setActiveCategory(activeCategory === cat ? null : cat)} + aria-pressed={activeCategory === cat} > {cat} </button> ))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/events.js` around lines 114 - 131, Add aria-pressed to the “All” button and each category button in the categories filter rendered by the events page. Bind it to the same selection state used by the active CSS classes: true when All is selected or the category matches activeCategory, and false otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/pages/events.js`:
- Around line 114-131: Add aria-pressed to the “All” button and each category
button in the categories filter rendered by the events page. Bind it to the same
selection state used by the active CSS classes: true when All is selected or the
category matches activeCategory, and false otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 581b1941-6277-4d32-87c3-6509837afa63
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/pages/events.jssrc/pages/events.module.csssrc/plugins/events/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/pages/events.module.css
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/pages/events.js:120
- The “All” category pill is a toggle, but it doesn’t expose its selected state to assistive tech. Add
aria-pressed(andtype="button") so screen readers can announce the active filter.
<button
className={`${styles.filterPill} ${activeCategory === null ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(null)}
>
{isZh ? "全部" : "All"}
src/pages/events.js:127
- Category pills behave like toggle buttons (clicking the active category clears the filter), but the pressed/unpressed state isn’t exposed. Add
aria-pressed(andtype="button") to each category button so the active filter is announced correctly.
<button
key={cat}
className={`${styles.filterPill} ${activeCategory === cat ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(activeCategory === cat ? null : cat)}
>
src/plugins/events/index.js:53
- The catch block logs only
err.message, which drops stack traces and can be undefined if a non-Error is thrown. Logging the error object preserves useful debugging information in CI/build logs.
} catch (err) {
console.warn(
`plugin-events: failed to parse "${src.name}" (${src.icsUrl}), skipping:`,
err.message,
);
Memoize the date window on weekOffset and today so eventsByDay and daysWithEvents actually cache between renders, and bucket events by computing their day key once instead of scanning all visible days per event. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/pages/events.js:113
- The category filter button is a toggle, but it doesn’t expose its pressed state to assistive tech. Consider adding
aria-pressed(and also explicitly settingtype="button"to match the project’s button usage) so screen readers can understand which filter is active.
<button
className={`${styles.filterPill} ${activeCategory === null ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(null)}
>
src/pages/events.js:121
- The per-category filter buttons are also toggles; without
aria-pressed, the selected state is only conveyed visually. Addingaria-pressedimproves accessibility. Also settype="button"to avoid implicit submit behavior.
<button
key={cat}
className={`${styles.filterPill} ${activeCategory === cat ? styles.filterPillActive : ""}`}
onClick={() => setActiveCategory(activeCategory === cat ? null : cat)}
>
Add type=button to the filter and navigation buttons for consistency with the rest of the codebase, and render a short fallback message during SSR and for no-JS visitors instead of an empty section. Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fishman, FouoF, windsonsea The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Co-authored-by: deepseek deepseek@users.noreply.github.com
Signed-off-by: Reza Jelveh fishmangit@dynamia.ai
What type of PR is this?
/kind documentation
/kind feature
What this PR does / why we need it:
This is based on the vLLM events page. https://vllm.ai/events
Summary by CodeRabbit