Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2dd9dc5
Initial plan
Copilot Sep 28, 2025
0625b99
feat(web): add SomedayRecurrenceSection with restricted frequency opt…
Copilot Sep 28, 2025
2b016d0
Merge branch 'main' into copilot/fix-8038056b-6260-4367-a757-556e9e0d…
tyler-dane Sep 29, 2025
90dc253
Merge branch 'main' into copilot/fix-8038056b-6260-4367-a757-556e9e0d…
tyler-dane Oct 4, 2025
bf20695
Merge branch 'main' into copilot/fix-8038056b-6260-4367-a757-556e9e0d…
tyler-dane Oct 4, 2025
049520d
refactor(web): replace ConditionalRender with conditional rendering i…
tyler-dane Oct 4, 2025
d349114
refactor(web): remove unused ConditionalRender component
tyler-dane Oct 4, 2025
87ec85e
refactor(web): remove end date controls from SomedayRecurrenceSection
tyler-dane Oct 4, 2025
8fd8279
refactor(web): modularize recurrence code
tyler-dane Oct 4, 2025
84a7620
test(web): add category prop to EventForm tests for better event cate…
tyler-dane Oct 4, 2025
99926f8
refactor(web): modularize RecurrenceSection children
tyler-dane Oct 4, 2025
b824dfd
refactor(web): enhance SomedayRecurrenceSection with improved select …
tyler-dane Oct 4, 2025
033f4c5
refactor(web): update SomedayRecurrenceSection to use role-based quer…
tyler-dane Oct 4, 2025
3c16d90
refactor(web): implement cancel edit functionality in SomedayRecurren…
tyler-dane Oct 4, 2025
35e918f
refactor(web): improve event handling in SomedayEventForm and add com…
tyler-dane Oct 5, 2025
1b5622c
style(web): update font size and text case in SomedayRecurrenceSectio…
tyler-dane Oct 5, 2025
d9dd486
style(web): adjust overflow handling in SomedayRecurrenceSelect compo…
tyler-dane Oct 5, 2025
aab04a3
Update packages/web/src/views/Forms/EventForm/DateControlsSection/Rec…
tyler-dane Oct 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Categories_Event } from "@core/types/event.types";
import {
DateTimeSection,
Props as DateTimeSectionProps,
} from "./DateTimeSection/DateTimeSection";
} from "../DateTimeSection/DateTimeSection";
import { StyledControlsSection } from "./styled";

interface Props {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { brighten, darken } from "@core/util/color.utils";
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

import { DatePicker } from "@web/components/DatePicker/DatePicker";
import { Flex } from "@web/components/Flex";
import { RepeatIcon } from "@web/components/Icons/Repeat";
Expand All @@ -24,13 +23,13 @@ import {
StyledRepeatTextContainer,
StyledWeekDay,
} from "@web/views/Forms/EventForm/DateControlsSection/RecurrenceSection/styled";
import { useRecurrence } from "@web/views/Forms/EventForm/DateControlsSection/RecurrenceSection/useRecurrence/useRecurrence";
import {
FREQUENCY_MAP,
FREQUENCY_OPTIONS,
FrequencyValues,
WEEKDAYS,
useRecurrence,
} from "@web/views/Forms/EventForm/DateControlsSection/RecurrenceSection/utils";
} from "./constants/recurrence.constants";

export interface RecurrenceSectionProps {
bgColor: string;
Expand Down Expand Up @@ -66,22 +65,20 @@ const RecurrenceToggle = ({
}) => {
return (
<StyledRepeatRow>
<ConditionalRender condition={!hasRecurrence}>
{!hasRecurrence ? (
<StyledRepeatContainer onClick={toggleRecurrence}>
<StyledRepeatText hasRepeat={hasRecurrence} tabIndex={0}>
Does not repeat
</StyledRepeatText>
</StyledRepeatContainer>
</ConditionalRender>

<ConditionalRender condition={hasRecurrence}>
) : (
<StyledRepeatTextContainer onClick={toggleRecurrence}>
<RepeatIcon />
<StyledRepeatText hasRepeat={hasRecurrence}>
Repeats every
</StyledRepeatText>
</StyledRepeatTextContainer>
</ConditionalRender>
)}
</StyledRepeatRow>
);
};
Expand Down Expand Up @@ -382,43 +379,45 @@ export const RecurrenceSection = ({
marginBottom: 10,
}}
>
<ConditionalRender condition={hasRecurrence}>
{hasRecurrence && (
<EditRecurrence
open={showForm}
onClick={() => setShowForm((value) => !value)}
/>
</ConditionalRender>
)}

<ConditionalRender condition={hasRecurrence ? showForm : true}>
{(!hasRecurrence || showForm) && (
<RecurrenceToggle
hasRecurrence={hasRecurrence}
toggleRecurrence={toggleRecurrence}
/>
</ConditionalRender>
)}

<ConditionalRender condition={hasRecurrence && showForm}>
<RecurrenceIntervalSelect
bgColor={bgColor}
initialValue={interval}
frequency={freq}
onChange={setInterval}
onFreqSelect={setFreq}
min={1}
max={12}
/>
{hasRecurrence && showForm && (
<>
<RecurrenceIntervalSelect
bgColor={bgColor}
initialValue={interval}
frequency={freq}
onChange={setInterval}
onFreqSelect={setFreq}
min={1}
max={12}
/>

<WeekDays bgColor={bgColor} value={weekDays} onChange={setWeekDays} />
<WeekDays bgColor={bgColor} value={weekDays} onChange={setWeekDays} />

<EndsOnDate
bgColor={bgColor}
inputColor={
hoverColorByPriority[event.priority ?? Priorities.UNASSIGNED]
}
until={until}
minDate={event.endDate}
setUntil={setUntil}
/>
</ConditionalRender>
<EndsOnDate
bgColor={bgColor}
inputColor={
hoverColorByPriority[event.priority ?? Priorities.UNASSIGNED]
}
until={until}
minDate={event.endDate}
setUntil={setUntil}
/>
</>
)}
</StyledRepeatRow>
);
};
Loading