Skip to content

Comments

feat(web): restrict someday event recurrence to weekly/monthly only#1017

Merged
tyler-dane merged 18 commits intomainfrom
copilot/fix-8038056b-6260-4367-a757-556e9e0d7ff4
Oct 5, 2025
Merged

feat(web): restrict someday event recurrence to weekly/monthly only#1017
tyler-dane merged 18 commits intomainfrom
copilot/fix-8038056b-6260-4367-a757-556e9e0d7ff4

Conversation

Copy link
Contributor

Copilot AI commented Sep 28, 2025

Problem

Someday event forms were allowing users to create daily recurring events, which doesn't make sense in the context of the someday list. The someday list organizes tasks according to weekly and monthly timeframes with a limit of 9 tasks per period. Daily recurrence would result in 7 identical tasks in the current week, breaking this organizational structure.

Solution

Created a specialized SomedayRecurrenceSection component that restricts recurrence options for someday events while maintaining all core functionality.
Screenshot 2025-10-04 at 8 33 17 PM

Fixes #1005

Related to #1058 <- this is happening in main and this branch, which means this bug wasn't introduced in this PR. Therefore, I'm moving foward with this


This pull request refactors the recurrence controls in the event form, primarily by removing the ConditionalRender component and replacing it with direct conditional rendering. It also introduces a new SomedayRecurrenceSection component with comprehensive tests, and reorganizes recurrence-related imports for better clarity and maintainability.

Refactoring and Code Simplification

  • Removed the ConditionalRender component and replaced all its usages in RecurrenceSection with direct conditional rendering, simplifying the logic and reducing unnecessary abstraction (conditional-render.tsx, RecurrenceSection.tsx). [1] [2] [3] [4]

Feature Addition and Testing

  • Added a new SomedayRecurrenceSection component for handling recurrence options for "someday" events, including logic for opening/closing the select menu, handling option selection, and canceling edits (SomedayRecurrenceSection.tsx).
  • Added a comprehensive test suite for SomedayRecurrenceSection, covering rendering, recurrence option selection, UI restrictions, keyboard interactions, and state restoration (SomedayRecurrenceSection.test.tsx).

Code Organization and Import Updates

  • Updated imports in recurrence-related files to use more specific paths and separated constants, hooks, and utility functions for improved clarity and maintainability (DateControlsSection.tsx, RecurrenceSection.tsx). [1] [2] [3]

…ions

Co-authored-by: tyler-dane <30163055+tyler-dane@users.noreply.github.com>
Copilot AI changed the title [WIP] Someday event form allows user to create recurring daily events feat(web): restrict someday event recurrence to weekly/monthly only Sep 28, 2025
Copilot AI requested a review from tyler-dane September 28, 2025 20:23
import { parseCompassEventDate } from "@core/util/event/event.util";
import { theme } from "@web/common/styles/theme";
import { hoverColorByPriority } from "@web/common/styles/theme.util";
import { ConditionalRender } from "@web/components/ConditionalRender/conditional-render";
Copy link
Contributor

Choose a reason for hiding this comment

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

This component wasn't providing any additional value or functionality from the standard JSX rendering. It also introduced a second way to render conditionally, which makes the codebase less consistent.

Copy link
Contributor

@victor-enogwe victor-enogwe Oct 9, 2025

Choose a reason for hiding this comment

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

@tyler-dane I didn't see this on time. This component is needed to ensure we have a cleaner rendering process.
It also allows us to replace the inconsistent rendering logic using && which relies on JS's short-circuiting evalutation which can sometimes produce unexpected results.

Copy link
Contributor

Choose a reason for hiding this comment

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

Summarizing an offline meeting:

Instead of using ConditonalRender we decided

  • to get away from ternary and nested ternary, we'll use a local conditional component, where it returns a component or null. It'll just use pure React, though (not a custom ConditionalRender component)

Pros & Cons:

Why ConditionalRender

  • reduces context switching between HTML and JSX
  • preference
  • easier to understand — avoids bugs with && and conditional
  • platform/version agnostic (runs same on web vs electron, eg)

Why not ConditionalRender

  • verbose
  • multiple patterns
  • raises barrier to entry for new contributors
  • doesn’t do anything that JS that JSX + variables can’t do

@tyler-dane tyler-dane marked this pull request as ready for review October 5, 2025 00:50
Copilot AI review requested due to automatic review settings October 5, 2025 00:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Introduces a specialized SomedayRecurrenceSection to restrict Someday event recurrence to weekly or monthly, removes UI elements not applicable to Someday events, and refactors recurrence utilities into clearer modules.

  • Adds Someday-specific recurrence select (weekly/monthly only) and removes daily option, interval, weekday, and end-date controls from Someday recurrence UI.
  • Refactors recurrence utilities into constants, util, and hook-specific files with expanded unit tests.
  • Adjusts keyboard handling to avoid form submission conflicts when interacting with recurrence comboboxes.

Reviewed Changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
SomedayEventForm/useSomedayFormShortcuts.ts Adds combobox interaction guard to Enter hotkey handling.
SomedayEventForm/useSomedayFormShortcuts.test.tsx Tests new combobox handling behavior.
SomedayEventForm/SomedayEventForm.tsx Integrates new SomedayRecurrenceSection and adjusts Enter handling.
EventForm/EventForm.tsx Updates import path for DateControlsSection.
EventForm/EventForm.test.tsx Passes category prop (potentially extraneous) into EventForm.
RecurrenceSection/* (new + refactor) Splits recurrence constants, utils, and hook tests; adds Someday recurrence components.
RecurrenceSection/util & constants tests Adds granular unit tests for recurrence transformations.
RecurrenceSection/useRecurrence/useRecurrence.ts Refactors hook; (introduces an incorrect type import).
RecurrenceSection/styled.ts Styling adjustments for recurrence UI affordances.
SomedayRecurrenceSection/* New Someday-specific recurrence selector components and tests.
RecurrenceSection/RecurrenceSection.tsx Removes ConditionalRender usage in favor of inline conditionals.
DateControlsSection/DateControlsSection.tsx Adjusts relative import path for DateTimeSection.
ConditionalRender/conditional-render.tsx Removes unused ConditionalRender component.

…urrenceSection/useRecurrence/useRecurrence.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@tyler-dane tyler-dane merged commit 9412f31 into main Oct 5, 2025
5 checks passed
@tyler-dane tyler-dane deleted the copilot/fix-8038056b-6260-4367-a757-556e9e0d7ff4 branch October 5, 2025 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Someday event form allows user to create recurring daily events

3 participants