Skip to content

feat(calendar): redesign app using plain HTML/CSS/TS#717

Draft
nicomiguelino wants to merge 1 commit intomasterfrom
feat/overhaul-calendar-app
Draft

feat(calendar): redesign app using plain HTML/CSS/TS#717
nicomiguelino wants to merge 1 commit intomasterfrom
feat/overhaul-calendar-app

Conversation

@nicomiguelino
Copy link
Contributor

@nicomiguelino nicomiguelino commented Mar 2, 2026

Summary

  • Migrate calendar app from Vue 3 + Pinia to framework-free Web Components
  • Add reusable <weekly-calendar-view> custom element to edge-apps-library
  • Port column layout algorithm and 12-hour window logic from the blueprint
  • Implement partial event clipping and column-scoped current-time indicator

@nicomiguelino @claude
feat(edge-apps): redesign calendar app using plain HTML/CSS/TS
7378c5c 
- Migrate from Vue 3 + Pinia to framework-free Web Components
- Add reusable weekly-calendar-view custom element to edge-apps-library

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nicomiguelino nicomiguelino self-assigned this Mar 2, 2026
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 Security concerns

SSRF / untrusted URL fetching:
fetchCalendarEventsFromICal constructs icalUrlWithProxy from screenly.settings.ical_url (a secret) and optionally prepends screenly.cors_proxy_url via string concatenation. If app settings can be influenced, this enables fetching arbitrary URLs (directly or via proxy), which is a common SSRF risk and may expose internal endpoints through the runtime/proxy. Mitigations to consider: validate ical_url scheme/host (allowlist or at least restrict to http/https), URL-encode the target when passing through the proxy, and ensure the proxy endpoint cannot be abused to reach internal networks.

⚡ Recommended focus areas for review

Date Filtering

The iCal fetcher filters events using only the event start timestamp within the computed view range. This can drop events that start before the range but overlap into it (e.g., multi-hour events crossing midnight or spanning into the day/week). Consider filtering by overlap (eventEnd > rangeStart && eventStart < rangeEnd) and validating behavior for all-day and multi-day events.

 
const mappedViewMode: ViewMode =
  viewMode === 'monthly' ? VIEW_MODE.SCHEDULE : (viewMode as ViewMode)
const { startDate, endDate } = getDateRangeForViewMode(
  mappedViewMode,
  timezone,
)

const startTimestamp = startDate.getTime()
const endTimestamp = endDate.getTime()

const chunkSize = 100
const events: CalendarEvent[] = []

for (let i = 0; i < vevents.length; i += chunkSize) {
  const chunk = vevents.slice(i, i + chunkSize)

  chunk.forEach((vevent) => {
    const event = new ical.Event(vevent)
    const eventStart = event.startDate.toJSDate()
    const eventTimestamp = eventStart.getTime()

    if (eventTimestamp >= endTimestamp || eventTimestamp < startTimestamp) {
      return
    }

    const eventEnd = event.endDate.toJSDate()

    events.push({
      title: event.summary || 'Busy',
      startTime: eventStart.toISOString(),
      endTime: eventEnd.toISOString(),
      isAllDay: event.startDate.isDate,
    })
  })
Robust Parsing

bypass_cors is parsed via JSON.parse on a settings value that may be undefined or not valid JSON, which can throw and fail the whole fetch. Also, concatenating the proxy URL and the iCal URL without encoding can produce malformed URLs (double slashes, missing scheme, etc.). Add safer parsing/defaulting for the setting and build the proxied URL defensively.

 
const screenlySettings = screenly.settings
const { ical_url: icalUrl } = screenlySettings
const corsProxy = screenly.cors_proxy_url
const bypassCors = Boolean(
  JSON.parse(screenlySettings.bypass_cors as string),
)
const viewMode = screenlySettings.calendar_mode as string

const icalUrlWithProxy = bypassCors
  ? `${corsProxy}/${icalUrl as string}`
  : (icalUrl as string)

const response = await fetch(icalUrlWithProxy)
Performance

The component rerenders the entire shadow DOM on every now update (set every second) and on any events/timezone/locale change by rebuilding all DOM nodes. This can be expensive on low-powered devices and may cause layout/paint churn. Consider minimizing updates (e.g., update only the current-time indicator position), caching computed layouts/timeSlots per window, or using a more incremental render strategy.

 
get now(): Date {
  return this._now
}

set now(value: Date) {
  this._now = value
  if (this._initialized) this._render()
}

private _getWeekStart(): Date {
  const tz = this._timezone
  const nowInTz = dayjs(this._now).tz(tz)
  const weekStart = nowInTz.startOf('week')
  return weekStart.toDate()
}

private _getEventLayouts(): Map<string, EventLayout> {
  const tz = this._timezone
  const weekStartDate = dayjs(this._getWeekStart()).tz(tz)
  const layoutMap = new Map<string, EventLayout>()
  const eventsByDay = new Map<number, CalendarEvent[]>()

  const weekEvents = this._events.filter((event) => {
    const eventStart = dayjs(event.startTime).tz(tz)
    return (
      !eventStart.isBefore(weekStartDate) &&
      eventStart.isBefore(weekStartDate.add(7, 'day'))
    )
  })

  weekEvents.forEach((event) => {
    const eventStart = dayjs(event.startTime).tz(tz)
    const dayDiff = eventStart.diff(weekStartDate, 'day')
    const dayIndex = ((dayDiff % 7) + 7) % 7

    if (!eventsByDay.has(dayIndex)) {
      eventsByDay.set(dayIndex, [])
    }
    eventsByDay.get(dayIndex)!.push(event)
  })

  for (let dayIndex = 0; dayIndex < 7; dayIndex++) {
    const dayEvents = eventsByDay.get(dayIndex) || []
    if (dayEvents.length === 0) continue

    const clusters = findEventClusters(dayEvents, tz)
    for (const cluster of clusters) {
      const clusterLayouts = calculateClusterLayouts(cluster, tz)
      for (const [event, layout] of clusterLayouts) {
        layoutMap.set(getEventKey(event), layout)
      }
    }
  }

  return layoutMap
}

private _render() {
  const shadow = this.shadowRoot!
  const tz = this._timezone
  const locale = this._locale
  const now = this._now

  const currentHour = parseInt(
    now.toLocaleString('en-US', {
      hour: 'numeric',
      hour12: false,
      timeZone: tz,
    }),
    10,
  )
  const windowStartHour = getWindowStartHour(currentHour)
  const timeSlots = generateTimeSlots(windowStartHour, now, locale)

  const weekStart = this._getWeekStart()
  const eventLayouts = this._getEventLayouts()

  const todayStr = dayjs(now).tz(tz).format('YYYY-MM-DD')

  const currentMinute = parseInt(
    now.toLocaleString('en-US', {
      minute: 'numeric',
      timeZone: tz,
    }),
    10,
  )
  const currentSlotIndex = timeSlots.findIndex(
    (slot) => slot.hour === currentHour,
  )
  const minuteFraction = currentMinute / 60
  const timeIndicatorPct =
    currentSlotIndex >= 0
      ? ((currentSlotIndex + minuteFraction) / 12) * 100
      : -1

  shadow.innerHTML = `<style>${componentCss}</style>`

  const container = document.createElement('div')
  container.className = 'weekly-calendar-container'

  const title = document.createElement('p')
  title.className = 'this-week-title'
  title.textContent = 'This week'
  container.appendChild(title)

  const weekGrid = document.createElement('div')
  weekGrid.className = 'week-grid'

  const timeGutter = document.createElement('div')
  timeGutter.className = 'time-gutter'
  for (const slot of timeSlots) {
    const label = document.createElement('div')
    label.className = 'time-label'
    label.textContent = slot.time
    timeGutter.appendChild(label)
  }
  weekGrid.appendChild(timeGutter)

  const daysGrid = document.createElement('div')
  daysGrid.className = 'days-grid'

  for (let dayIdx = 0; dayIdx < 7; dayIdx++) {
    const dayDate = new Date(weekStart)
    dayDate.setDate(dayDate.getDate() + dayIdx)
    const dayDateStr = dayjs(dayDate).tz(tz).format('YYYY-MM-DD')
    const isToday = dayDateStr === todayStr

    const dayCol = document.createElement('div')
    dayCol.className = 'day-column'
    setAttribute(dayCol, 'data-day-index', String(dayIdx))

    const dayHeader = document.createElement('div')
    dayHeader.className = isToday ? 'day-header today' : 'day-header'

    const dayName = document.createElement('span')
    dayName.className = 'day-name'
    dayName.textContent = DAYS_OF_WEEK[dayIdx] || ''
    dayHeader.appendChild(dayName)

    const dayDateNum = document.createElement('span')
    dayDateNum.className = 'day-date'
    dayDateNum.textContent = String(dayDate.getDate())
    dayHeader.appendChild(dayDateNum)

    dayCol.appendChild(dayHeader)

    const dayBody = document.createElement('div')
    dayBody.className = 'day-body'

    for (let rowIdx = 0; rowIdx < 12; rowIdx++) {
      const hourRow = document.createElement('div')
      hourRow.className = 'hour-row'
      dayBody.appendChild(hourRow)
    }

    const windowEndHour = (windowStartHour + 12) % 24

    const dayEvents = this._events.filter((event) => {
      if (event.isAllDay) return false
      const eventStart = dayjs(event.startTime).tz(tz)
      const eventDate = eventStart.format('YYYY-MM-DD')
      if (eventDate !== dayDateStr) return false

      const startH = eventStart.hour() + eventStart.minute() / 60
      const endDt = dayjs(event.endTime).tz(tz)
      const endH = endDt.hour() + endDt.minute() / 60

      const normalizedWindowEnd =
        windowEndHour <= windowStartHour
          ? windowEndHour + 24
          : windowEndHour
      const normalizedStart =
        startH < windowStartHour ? startH + 24 : startH
      const normalizedEnd =
        endH <= windowStartHour ? endH + 24 : endH

      return normalizedStart < normalizedWindowEnd && normalizedEnd > windowStartHour
    })

    for (const event of dayEvents) {
      const key = getEventKey(event)
      const layout = eventLayouts.get(key) ?? {
        event,
        column: 0,
        columnSpan: 1,
        totalColumns: 1,
      }

      const style = getEventStyle(event, windowStartHour, layout, tz)

      const wrapper = document.createElement('div')
      wrapper.className = 'event-wrapper'
      wrapper.style.setProperty('top', `${style.topPct}%`)
      wrapper.style.setProperty('height', `${style.heightPct}%`)
      wrapper.style.setProperty('width', `${style.widthPct}%`)
      wrapper.style.setProperty('left', `${style.leftPct}%`)
      wrapper.style.setProperty('z-index', String(style.zIndex))

      const item = document.createElement('div')
      const itemClasses = ['event-item']
      if (style.clippedTop) itemClasses.push('clipped-top')
      if (style.clippedBottom) itemClasses.push('clipped-bottom')
      item.className = itemClasses.join(' ')

      if (event.backgroundColor) {
        item.style.setProperty('background-color', event.backgroundColor)
      }

      const titleEl = document.createElement('div')
      titleEl.className = 'event-title'
      titleEl.textContent = event.title

      const timeEl = document.createElement('div')
      timeEl.className = 'event-time'
      timeEl.textContent = formatEventTime(
        event.startTime,
        event.endTime,
        locale,
        tz,
      )

      item.appendChild(titleEl)
      item.appendChild(timeEl)
      wrapper.appendChild(item)
      dayBody.appendChild(wrapper)
    }

    if (isToday && timeIndicatorPct >= 0 && timeIndicatorPct <= 100) {
      const indicator = document.createElement('div')
      indicator.className = 'current-time-indicator'
      indicator.style.setProperty('top', `${timeIndicatorPct}%`)
      dayBody.appendChild(indicator)
    }

    dayCol.appendChild(dayBody)
    daysGrid.appendChild(dayCol)
  }

  weekGrid.appendChild(daysGrid)
  container.appendChild(weekGrid)
  shadow.appendChild(container)
}

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Include overlapping events in range

The current range filter only includes events whose start is inside the range,
which drops events that start before the window but overlap into it (common for long
meetings). Filter by range overlap (start < endDate && end > startDate) so ongoing
events are included.

edge-apps/calendar-new/src/events.ts [83-99]

 const event = new ical.Event(vevent)
 const eventStart = event.startDate.toJSDate()
-const eventTimestamp = eventStart.getTime()
+const eventEnd = event.endDate?.toJSDate?.() ?? eventStart
 
-if (eventTimestamp >= endTimestamp || eventTimestamp < startTimestamp) {
+const eventStartTs = eventStart.getTime()
+const eventEndTs = eventEnd.getTime()
+
+// Include events that overlap the requested window
+if (eventStartTs >= endTimestamp || eventEndTs <= startTimestamp) {
   return
 }
-
-const eventEnd = event.endDate.toJSDate()
 
 events.push({
   title: event.summary || 'Busy',
   startTime: eventStart.toISOString(),
   endTime: eventEnd.toISOString(),
   isAllDay: event.startDate.isDate,
 })
Suggestion importance[1-10]: 8

__

Why: The current filter only checks whether an event’s start time falls within [startDate, endDate), which incorrectly drops events that begin before the window but overlap into it. The proposed overlap check fixes a real correctness issue in fetchCalendarEventsFromICal() and the improved_code reflects the intended logic.

Medium
Avoid layout key collisions

Using getEventKey() can collide when multiple events share the same start/end/title,
causing incorrect layout lookups (overlaps rendered wrong). Store layouts keyed by
the event object reference instead (e.g., WeakMap<CalendarEvent, EventLayout>),
since the same objects are used during the render pass.

edge-apps/edge-apps-library/src/components/weekly-calendar-view/weekly-calendar-view.ts [5-355]

 import {
   type CalendarEvent,
   type EventLayout,
   findEventClusters,
   calculateClusterLayouts,
-  getEventKey,
 } from './event-layout.js'
 ...
-private _getEventLayouts(): Map<string, EventLayout> {
+private _getEventLayouts(): WeakMap<CalendarEvent, EventLayout> {
   const tz = this._timezone
   const weekStartDate = dayjs(this._getWeekStart()).tz(tz)
-  const layoutMap = new Map<string, EventLayout>()
+  const layoutMap = new WeakMap<CalendarEvent, EventLayout>()
   ...
-        layoutMap.set(getEventKey(event), layout)
+        layoutMap.set(event, layout)
   ...
   return layoutMap
 }
 ...
-    const key = getEventKey(event)
-    const layout = eventLayouts.get(key) ?? {
+    const layout = eventLayouts.get(event) ?? {
       event,
       column: 0,
       columnSpan: 1,
       totalColumns: 1,
     }
Suggestion importance[1-10]: 7

__

Why: Keying layouts by getEventKey() can collide when multiple events share the same startTime/endTime/title, causing eventLayouts.get(key) to return the wrong EventLayout. Switching to a WeakMap<CalendarEvent, EventLayout> keyed by the event object reference avoids collisions and fits the current flow where the same event objects are passed through clustering and rendering.

Medium
Reduce costly rerenders frequency

Updating now every second forces a full calendar re-render each second (the
component rebuilds the entire DOM), which can cause unnecessary CPU usage and jank.
Update on minute boundaries instead, which is sufficient for the time indicator and
dramatically reduces work.

edge-apps/calendar-new/src/main.ts [27-32]

 const tick = () => {
   calendarEl.now = new Date()
 }
 tick()
-setInterval(tick, 1000)
 
+const scheduleNextMinuteTick = () => {
+  const now = new Date()
+  const msUntilNextMinute =
+    (60 - now.getSeconds()) * 1000 - now.getMilliseconds()
+
+  setTimeout(() => {
+    tick()
+    setInterval(tick, 60_000)
+  }, msUntilNextMinute)
+}
+
+scheduleNextMinuteTick()
+
Suggestion importance[1-10]: 6

__

Why: Updating calendarEl.now every second triggers WeeklyCalendarView’s full _render() each second, which is unnecessarily expensive since the UI only needs minute-level updates for the time indicator. The suggested minute-boundary scheduling is a reasonable performance improvement, though not strictly required for correctness.

Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nicomiguelino nicomiguelino

Labels

Review effort 4/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nicomiguelino