From 784ffbcd005164d51bc562f1ab10110d6edbe2c6 Mon Sep 17 00:00:00 2001 From: Mohamad Tarbin Date: Tue, 4 Aug 2026 21:22:09 -0400 Subject: [PATCH] Fix action button layout and enhance notification handling (#190) * Fix approval /declien button issue. update action buttons and improve layout * feat(notifications): enhance notification handling and default templates across components --- src/components/NotificationTemplate.jsx | 12 ++- src/views/Chores/ChoreCard.jsx | 97 +++++++++---------- .../Chores/LocalNotificationScheduler.js | 20 +++- src/views/components/AddTaskModal.jsx | 84 +++++++++++++--- .../components/NotificationPickerField.jsx | 3 +- .../components/VoiceToTask/VoicePanel.jsx | 58 ++++++++++- .../components/VoiceToTask/parseVoiceTask.js | 19 +++- 7 files changed, 212 insertions(+), 81 deletions(-) diff --git a/src/components/NotificationTemplate.jsx b/src/components/NotificationTemplate.jsx index 9f468fa..73013b6 100644 --- a/src/components/NotificationTemplate.jsx +++ b/src/components/NotificationTemplate.jsx @@ -63,6 +63,10 @@ function getInternalValue(timing, displayValue) { const NotificationTemplate = ({ maxNotifications = 5, + // ChoreEdit gates this editor behind its own on/off switch, so the last row + // must stay put — `notification: true` with no templates is not a valid task. + // Consumers that own an empty state themselves pass 0. + minNotifications = 1, onChange, value, showTimeline = true, @@ -259,7 +263,8 @@ const NotificationTemplate = ({ return next }) - onChange && onChange(updated) + // No direct onChange here: consumers expect { notifications }, and the + // effect below already emits that shape once the state settles. setShowSaveDefault(true) } @@ -307,9 +312,6 @@ const NotificationTemplate = ({ return ( - - Notification Timeline - removeNotification(idx)} - disabled={notifications.length === 1} + disabled={notifications.length <= minNotifications} color={'danger'} size={'sm'} variant={'soft'} diff --git a/src/views/Chores/ChoreCard.jsx b/src/views/Chores/ChoreCard.jsx index 930de70..cf04f64 100644 --- a/src/views/Chores/ChoreCard.jsx +++ b/src/views/Chores/ChoreCard.jsx @@ -5,7 +5,7 @@ import { Pause, PlayArrow, Repeat, - Schedule, + ThumbDown, ThumbUp, TimesOneMobiledata, Toll, @@ -21,6 +21,7 @@ import { IconButton, Typography, } from '@mui/joy' + import { useImpersonateUser } from '../../contexts/ImpersonateUserContext.jsx' import { useLocalization } from '../../contexts/LocalizationContext' import { usePendingCommands } from '../../hooks/usePendingCommands' @@ -37,16 +38,16 @@ import ChoreActionMenu from '../components/ChoreActionMenu' import PendingBadge from '../components/PendingBadge' const ChoreCard = ({ chore, - performers, - sx, - viewOnly, - showActions = true, - onChipClick, - onAction, - // Multi-select props isMultiSelectMode = false, isSelected = false, + onAction, + onChipClick, onSelectionToggle, + performers, + // Multi-select props + showActions = true, + sx, + viewOnly, }) => { const { data: userProfile } = useUserProfile() const { timeFormat } = useLocalization() @@ -359,27 +360,6 @@ const ChoreCard = ({ justifyContent: 'center', }} > - {chore.status === 3 && ( - - - Pending - - )} {showActions && ( - {/* { - e.stopPropagation() - onAction('reject', chore) - }} - sx={{ - borderRadius: '50%', - minWidth: 40, - height: 40, - zIndex: 1, - transition: 'all 0.2s ease', - '&:hover': { - transform: 'scale(1.05)', - }, - '&:active': { - transform: 'scale(0.95)', - }, - }} - > - - */} + { + e.stopPropagation() + onAction('reject', chore) + }} + sx={{ + borderRadius: '50%', + width: 50, + minWidth: 50, + height: 50, + flexShrink: 0, + zIndex: 1, + transition: 'all 0.2s ease', + '&:hover': { + transform: 'scale(1.05)', + }, + '&:active': { + transform: 'scale(0.95)', + }, + }} + > + + ) : ( )} onAction('completeWithNote', chore) @@ -530,6 +513,16 @@ const ChoreCard = ({ onWriteNFC={() => onAction('writeNFC', chore)} onNudge={() => onAction('nudge', chore)} onDelete={() => onAction('delete', chore)} + sx={{ + width: 32, + height: 32, + color: 'text.tertiary', + flexShrink: 0, + '&:hover': { + color: 'text.secondary', + bgcolor: 'background.level1', + }, + }} /> )} diff --git a/src/views/Chores/LocalNotificationScheduler.js b/src/views/Chores/LocalNotificationScheduler.js index d5f5572..6642ac1 100644 --- a/src/views/Chores/LocalNotificationScheduler.js +++ b/src/views/Chores/LocalNotificationScheduler.js @@ -49,6 +49,23 @@ const getTimeFromTemplate = (template, relativeTime) => { } return time } +// Decide whether this device's user should be notified about a chore: +// - assignedTo set -> only that user +// - no assignedTo -> everyone listed in assignees +// - no assignees -> "Anyone" mode, notify the whole circle +const shouldNotifyUser = (chore, userId) => { + if (!userId) { + return false + } + if (chore.assignedTo > 0) { + return chore.assignedTo === userId + } + if (chore.assignees?.length > 0) { + return chore.assignees.some(assignee => assignee.userId === userId) + } + return true +} + const scheduleNotificationFromTemplate = ( chore, userProfile, @@ -193,7 +210,8 @@ const scheduleChoreNotification = async ( if ( chore.notification === false || chore.nextDueDate === null || - chore.isActive === false + chore.isActive === false || + !shouldNotifyUser(chore, userProfile?.id) ) { continue } diff --git a/src/views/components/AddTaskModal.jsx b/src/views/components/AddTaskModal.jsx index bbcc981..5f6270a 100644 --- a/src/views/components/AddTaskModal.jsx +++ b/src/views/components/AddTaskModal.jsx @@ -45,22 +45,65 @@ import ScanPanel from './ScanToTask/ScanPanel' import SubTasks from './SubTask' import { buildChorePayload, parseVoiceTask } from './VoiceToTask/parseVoiceTask' import VoicePanel from './VoiceToTask/VoicePanel' +// Canonical reminder template shape, shared with NotificationTemplate and +// LocalNotificationScheduler: a signed value plus 'm' | 'h' | 'd'. Negative is +// before due, positive is after, zero is on due. +const DEFAULT_NOTIFICATION_TEMPLATES = [ + { value: -1, unit: 'd' }, + { value: 0, unit: 'm' }, + { value: 1, unit: 'd' }, +] + +const UNIT_ALIASES = { + minute: 'm', + minutes: 'm', + hour: 'h', + hours: 'h', + day: 'd', + days: 'd', +} + +// Earlier builds stored {value: 1, unit: 'days', type: 'before'}. Nothing reads +// `type`, and the scheduler's unit switch falls through on 'days', so those +// entries fired at the due time (or collided on id) instead of offsetting. +const normalizeTemplate = template => { + const unit = UNIT_ALIASES[template.unit] || template.unit + const value = Number(template.value) || 0 + if (!template.type) return { value, unit } + if (template.type === 'ondue') return { value: 0, unit } + return { + value: template.type === 'before' ? -Math.abs(value) : Math.abs(value), + unit, + } +} + const getDefaultNotification = () => { const storedDefault = localStorage.getItem('defaultNotificationTemplate') if (storedDefault) { - return JSON.parse(storedDefault) + try { + const parsed = JSON.parse(storedDefault) + if (Array.isArray(parsed)) { + // An empty list is a deliberate "no reminders by default", not a + // missing value — respect it instead of re-seeding. + const normalized = parsed.map(normalizeTemplate) + if (JSON.stringify(normalized) !== storedDefault) { + localStorage.setItem( + 'defaultNotificationTemplate', + JSON.stringify(normalized), + ) + } + return normalized + } + } catch { + // fall through and reset to the defaults below + } } - const defaultNotification = [ - { value: 1, unit: 'days', type: 'before' }, - { value: 0, unit: 'minutes', type: 'ondue' }, - { value: 1, unit: 'days', type: 'after' }, - ] localStorage.setItem( 'defaultNotificationTemplate', - JSON.stringify(defaultNotification), + JSON.stringify(DEFAULT_NOTIFICATION_TEMPLATES), ) - return defaultNotification + return DEFAULT_NOTIFICATION_TEMPLATES } const TaskInput = ({ onChoreUpdate, isModalOpen, onClose, initialMode }) => { @@ -157,6 +200,12 @@ const TaskInput = ({ onChoreUpdate, isModalOpen, onClose, initialMode }) => { isListening: false, }) const [creatingVoiceTasks, setCreatingVoiceTasks] = useState(false) + // Reminder default the voice cards start from — read once so the array + // identity stays stable across renders of the panel + const voiceDefaultNotificationTemplates = useMemo( + () => getDefaultNotification(), + [], + ) // Same arrangement for the scan panel: it reports the action for its // current phase and the modal footer renders it const [scanState, setScanState] = useState({ @@ -558,6 +607,11 @@ const TaskInput = ({ onChoreUpdate, isModalOpen, onClose, initialMode }) => { if ('priority' in overrides) setPriority(overrides.priority || 0) if ('frequency' in overrides) setFrequency(overrides.frequency) if ('labelIds' in overrides) setLabelsV2(overrides.labelIds || []) + if ('notificationMetadata' in overrides) { + setNotificationMetadata( + overrides.notificationMetadata || { templates: [] }, + ) + } if ('assignees' in overrides || 'isAnyone' in overrides) { setIsAnyoneTask(!!overrides.isAnyone) setAssignees(overrides.assignees || []) @@ -812,24 +866,29 @@ const TaskInput = ({ onChoreUpdate, isModalOpen, onClose, initialMode }) => { status: 0, frequencyType: 'once', frequencyMetadata: {}, + notification: false, notificationMetadata: {}, subTasks: subTasks?.length > 0 ? subTasks : null, projectId: projectId === 'default' ? null : projectId, draftId: draftId, } + // Reminders are a Plus feature and only make sense when the user kept at + // least one template; without the flag the backend never schedules them. + const hasReminders = + isPlusAccount(userProfile) && notificationMetadata?.templates?.length > 0 + if (frequency) { chore.frequencyType = frequency.frequencyType chore.frequencyMetadata = frequency.frequencyMetadata chore.frequency = frequency.frequency - if (isPlusAccount(userProfile)) { - chore.notification = true - chore.notificationMetadata = notificationMetadata - } } if (!frequency && dueDate) { // Use RFC3339/ISO-8601 format expected by backend. chore.nextDueDate = new Date(dueDate).toISOString() + } + if (hasReminders && (frequency || dueDate)) { + chore.notification = true chore.notificationMetadata = notificationMetadata } @@ -1291,6 +1350,7 @@ const TaskInput = ({ onChoreUpdate, isModalOpen, onClose, initialMode }) => { userLabels={voiceLabels} members={voiceMembers} userProfile={userProfile} + defaultNotificationTemplates={voiceDefaultNotificationTemplates} onStateChange={setVoiceState} /> )} diff --git a/src/views/components/NotificationPickerField.jsx b/src/views/components/NotificationPickerField.jsx index f78ff05..b3ca8b2 100644 --- a/src/views/components/NotificationPickerField.jsx +++ b/src/views/components/NotificationPickerField.jsx @@ -135,10 +135,11 @@ const NotificationPickerField = ({ > { latestTemplatesRef.current = notifications }} - showTimeline + showTimeline={false} /> diff --git a/src/views/components/VoiceToTask/VoicePanel.jsx b/src/views/components/VoiceToTask/VoicePanel.jsx index f10cae2..be8d808 100644 --- a/src/views/components/VoiceToTask/VoicePanel.jsx +++ b/src/views/components/VoiceToTask/VoicePanel.jsx @@ -6,6 +6,7 @@ import { GraphicEq, Lock, Mic, + NotificationsNone, Person, Repeat, Sell, @@ -16,9 +17,11 @@ import { Box, Button, Chip, IconButton, Input, Typography } from '@mui/joy' import moment from 'moment' import { useEffect, useMemo, useRef, useState } from 'react' import { TASK_COLOR } from '../../../utils/Colors' +import { isPlusAccount } from '../../../utils/Helpers' import AssigneePickerField from '../AssigneePickerField' import DueDatePickerField from '../DueDatePickerField' import LabelsPickerField from '../LabelsPickerField' +import NotificationPickerField from '../NotificationPickerField' import PriorityPickerField from '../PriorityPickerField' import RepeatPickerField from '../RepeatPickerField' import { parseVoiceTask } from './parseVoiceTask' @@ -102,7 +105,11 @@ const describeFrequency = f => { return names[f.frequencyType] || 'Repeats' } -const buildChips = (effective, frequencyLabel, { members, currentUserId }) => { +const buildChips = ( + effective, + frequencyLabel, + { members, currentUserId, canRemind }, +) => { const chips = [] if (effective.dueDate) { chips.push({ @@ -128,6 +135,21 @@ const buildChips = (effective, frequencyLabel, { members, currentUserId }) => { label: `P${effective.priority}`, }) } + // Mirrors buildChorePayload: reminders only reach the backend for Plus + // accounts on a task that has something to remind against + const reminderCount = effective.notificationMetadata?.templates?.length || 0 + if ( + canRemind && + reminderCount > 0 && + (effective.dueDate || effective.frequency) + ) { + chips.push({ + key: 'reminders', + color: 'neutral', + icon: , + label: reminderCount > 1 ? `${reminderCount} reminders` : '1 reminder', + }) + } if (effective.points != null) { chips.push({ key: 'points', @@ -184,9 +206,17 @@ const TaskPreviewCard = ({ [segment.text, parseCtx], ) const overrides = useMemo(() => segment.overrides || {}, [segment.overrides]) + // The parser has no notion of reminders, so the account default stands in + // until the card overrides it — same fallback buildChorePayload applies const effective = useMemo( - () => ({ ...parsed, ...overrides }), - [parsed, overrides], + () => ({ + ...parsed, + notificationMetadata: { + templates: parseCtx.defaultNotificationTemplates || [], + }, + ...overrides, + }), + [parsed, overrides, parseCtx.defaultNotificationTemplates], ) const frequencyLabel = @@ -329,6 +359,7 @@ const TaskPreviewCard = ({ {expanded && ( onPatch({ labelIds: [] })} labels={parseCtx.userLabels} /> + {parseCtx.canRemind && ( + onPatch({ notificationMetadata: metadata })} + onClear={() => + onPatch({ notificationMetadata: { templates: [] } }) + } + /> + )} )} @@ -412,6 +453,7 @@ const VoicePanel = ({ userLabels = [], members = [], userProfile, + defaultNotificationTemplates = [], onStateChange, }) => { const { @@ -430,8 +472,14 @@ const VoicePanel = ({ const segmentsScrollRef = useRef(null) const parseCtx = useMemo( - () => ({ userLabels, members, currentUserId: userProfile?.id }), - [userLabels, members, userProfile?.id], + () => ({ + userLabels, + members, + currentUserId: userProfile?.id, + canRemind: isPlusAccount(userProfile), + defaultNotificationTemplates, + }), + [userLabels, members, userProfile, defaultNotificationTemplates], ) const partialParsed = useMemo( diff --git a/src/views/components/VoiceToTask/parseVoiceTask.js b/src/views/components/VoiceToTask/parseVoiceTask.js index ad46871..a4fb394 100644 --- a/src/views/components/VoiceToTask/parseVoiceTask.js +++ b/src/views/components/VoiceToTask/parseVoiceTask.js @@ -177,24 +177,33 @@ export const buildChorePayload = ( status: 0, frequencyType: 'once', frequencyMetadata: {}, + notification: false, notificationMetadata: {}, subTasks: null, projectId: projectId === 'default' ? null : projectId, draftId: generateUUID(), } + // A per-task override from the voice card wins over the account default; + // an override of [] is a deliberate "no reminders", not a missing value. + const templates = + parsed.notificationMetadata?.templates ?? notificationTemplates + + // Reminders are a Plus feature; the flag is what makes the backend schedule + // them, so metadata alone is not enough. + const hasReminders = isPlusAccount(userProfile) && templates?.length > 0 + if (parsed.frequency) { chore.frequencyType = parsed.frequency.frequencyType chore.frequencyMetadata = parsed.frequency.frequencyMetadata chore.frequency = parsed.frequency.frequency - if (isPlusAccount(userProfile)) { - chore.notification = true - chore.notificationMetadata = { templates: notificationTemplates } - } } if (!parsed.frequency && parsed.dueDate) { chore.nextDueDate = new Date(parsed.dueDate).toISOString() - chore.notificationMetadata = { templates: notificationTemplates } + } + if (hasReminders && (parsed.frequency || parsed.dueDate)) { + chore.notification = true + chore.notificationMetadata = { templates } } return chore