From 9059630345faaf272151d29a5c32c4c74519898e Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Fri, 19 Dec 2025 14:50:34 -0600 Subject: [PATCH 1/7] fix navigation issues and project loading issues --- eslint.config.js | 1 + .../src/primitives/useProject/checklists.js | 11 ++- .../stores/projectActionsStore/checklists.js | 12 ++- packages/web/src/stores/projectStore.js | 74 ++++++++++++------- 4 files changed, 68 insertions(+), 30 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index e39625e07..c216584c1 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -41,6 +41,7 @@ export default [ document: 'readonly', navigator: 'readonly', console: 'readonly', + fetch: 'readonly', URL: 'readonly', caches: 'readonly', diff --git a/packages/web/src/primitives/useProject/checklists.js b/packages/web/src/primitives/useProject/checklists.js index 23cb6d864..d91d06968 100644 --- a/packages/web/src/primitives/useProject/checklists.js +++ b/packages/web/src/primitives/useProject/checklists.js @@ -90,6 +90,9 @@ export function createChecklistOperations(projectId, getYDoc, isSynced) { // Store answers as a Y.Map const answersYMap = new Y.Map(); + // Add answersYMap to checklistYMap early so it's part of the document structure + // This prevents Yjs errors when accessing the map + checklistYMap.set('answers', answersYMap); if (type === CHECKLIST_TYPES.AMSTAR2) { // AMSTAR2: Store each question as a nested Y.Map with answers, critical, and note @@ -98,6 +101,9 @@ export function createChecklistOperations(projectId, getYDoc, isSynced) { const multiPartParents = ['q9', 'q11']; const subQuestionPattern = /^(q9|q11)[a-z]$/; + // Track which keys we've added to avoid checking with .has() before map is in document + const addedKeys = new Set(); + Object.entries(answersData).forEach(([questionKey, questionData]) => { const questionYMap = new Y.Map(); questionYMap.set('answers', questionData.answers); @@ -110,12 +116,13 @@ export function createChecklistOperations(projectId, getYDoc, isSynced) { } answersYMap.set(questionKey, questionYMap); + addedKeys.add(questionKey); }); // Add note entries for multi-part parent questions (q9, q11) // These don't have answer data but need a note multiPartParents.forEach(parentKey => { - if (!answersYMap.has(parentKey)) { + if (!addedKeys.has(parentKey)) { const parentYMap = new Y.Map(); parentYMap.set('note', new Y.Text()); answersYMap.set(parentKey, parentYMap); @@ -181,8 +188,6 @@ export function createChecklistOperations(projectId, getYDoc, isSynced) { }); } - checklistYMap.set('answers', answersYMap); - checklistsMap.set(checklistId, checklistYMap); // Update study's updatedAt diff --git a/packages/web/src/stores/projectActionsStore/checklists.js b/packages/web/src/stores/projectActionsStore/checklists.js index cff943c6c..8d82d0529 100644 --- a/packages/web/src/stores/projectActionsStore/checklists.js +++ b/packages/web/src/stores/projectActionsStore/checklists.js @@ -76,8 +76,16 @@ export function createChecklistActions(getActiveConnection) { * Get checklist data */ function getData(studyId, checklistId) { - const ops = getActiveConnection(); - return ops?.getChecklistData?.(studyId, checklistId); + try { + const ops = getActiveConnection(); + return ops?.getChecklistData?.(studyId, checklistId); + } catch (err) { + // Handle case where there's no active project (e.g., during navigation) + if (err.message?.includes('No active project')) { + return null; + } + throw err; + } } /** diff --git a/packages/web/src/stores/projectStore.js b/packages/web/src/stores/projectStore.js index e567e2b16..bf233c2c4 100644 --- a/packages/web/src/stores/projectStore.js +++ b/packages/web/src/stores/projectStore.js @@ -11,6 +11,7 @@ import { API_BASE } from '@config/api.js'; // LocalStorage keys for offline caching const PROJECT_LIST_CACHE_KEY = 'corates-project-list-cache'; const PROJECT_LIST_CACHE_TIMESTAMP_KEY = 'corates-project-list-cache-timestamp'; +const PROJECT_LIST_CACHE_USER_ID_KEY = 'corates-project-list-cache-user-id'; const PROJECT_LIST_CACHE_MAX_AGE = 7 * 24 * 60 * 60 * 1000; // 7 days // Temporary in-memory storage for pending uploads during project creation @@ -28,6 +29,7 @@ function createProjectStore() { try { const cached = localStorage.getItem(PROJECT_LIST_CACHE_KEY); const timestamp = localStorage.getItem(PROJECT_LIST_CACHE_TIMESTAMP_KEY); + const cachedUserId = localStorage.getItem(PROJECT_LIST_CACHE_USER_ID_KEY); if (!cached || !timestamp) return null; const age = Date.now() - parseInt(timestamp, 10); @@ -35,10 +37,11 @@ function createProjectStore() { // Cache expired, clear it localStorage.removeItem(PROJECT_LIST_CACHE_KEY); localStorage.removeItem(PROJECT_LIST_CACHE_TIMESTAMP_KEY); + localStorage.removeItem(PROJECT_LIST_CACHE_USER_ID_KEY); return null; } - return JSON.parse(cached); + return { projects: JSON.parse(cached), userId: cachedUserId }; } catch (err) { console.error('Error loading cached project list:', err); return null; @@ -46,15 +49,17 @@ function createProjectStore() { } // Save project list to localStorage - function saveCachedProjectList(projects) { + function saveCachedProjectList(projects, userId) { if (typeof window === 'undefined') return; try { - if (projects && Array.isArray(projects)) { + if (projects && Array.isArray(projects) && userId) { localStorage.setItem(PROJECT_LIST_CACHE_KEY, JSON.stringify(projects)); localStorage.setItem(PROJECT_LIST_CACHE_TIMESTAMP_KEY, Date.now().toString()); + localStorage.setItem(PROJECT_LIST_CACHE_USER_ID_KEY, userId); } else { localStorage.removeItem(PROJECT_LIST_CACHE_KEY); localStorage.removeItem(PROJECT_LIST_CACHE_TIMESTAMP_KEY); + localStorage.removeItem(PROJECT_LIST_CACHE_USER_ID_KEY); } } catch (err) { console.error('Error saving cached project list:', err); @@ -62,20 +67,22 @@ function createProjectStore() { } // Initialize with cached data if available - const cachedProjectList = loadCachedProjectList(); + const cachedData = loadCachedProjectList(); const initialProjectList = - cachedProjectList ? + cachedData?.projects ? { - items: cachedProjectList, - loaded: true, // Mark as loaded so we show cached data immediately + items: cachedData.projects, + loaded: false, // Don't mark as loaded - we'll check userId and fetch if needed loading: false, error: null, + cachedUserId: cachedData.userId, // Track which user this cache belongs to } : { items: [], loaded: false, loading: false, error: null, + cachedUserId: null, }; const [store, setStore] = createStore({ @@ -299,8 +306,8 @@ function createProjectStore() { s.projectList.items.push(project); }), ); - // Update cache - saveCachedProjectList(store.projectList.items); + // Update cache (preserve userId) + saveCachedProjectList(store.projectList.items, store.projectList.cachedUserId); } /** @@ -315,8 +322,8 @@ function createProjectStore() { } }), ); - // Update cache - saveCachedProjectList(store.projectList.items); + // Update cache (preserve userId) + saveCachedProjectList(store.projectList.items, store.projectList.cachedUserId); } /** @@ -328,8 +335,8 @@ function createProjectStore() { s.projectList.items = s.projectList.items.filter(p => p.id !== projectId); }), ); - // Update cache - saveCachedProjectList(store.projectList.items); + // Update cache (preserve userId) + saveCachedProjectList(store.projectList.items, store.projectList.cachedUserId); } /** @@ -341,9 +348,10 @@ function createProjectStore() { loaded: false, loading: false, error: null, + cachedUserId: null, }); // Clear cached data - saveCachedProjectList(null); + saveCachedProjectList(null, null); } /** @@ -361,8 +369,21 @@ function createProjectStore() { async function fetchProjectList(userId, options = {}) { const { force = false } = options; - // Skip if already loaded (unless forcing refresh) - if (!force && store.projectList.loaded) { + // Check if cached data belongs to a different user - if so, force refresh and clear cache + const cachedUserId = store.projectList.cachedUserId; + const shouldForceRefresh = force || (cachedUserId && cachedUserId !== userId); + + // If userId changed, clear the cached items + if (cachedUserId && cachedUserId !== userId) { + setStore('projectList', { + items: [], + loaded: false, + cachedUserId: null, + }); + } + + // Skip if already loaded and userId matches (unless forcing refresh) + if (!shouldForceRefresh && store.projectList.loaded && cachedUserId === userId) { return store.projectList.items; } @@ -375,18 +396,19 @@ function createProjectStore() { return []; } - // Check if we're offline - if so, try to use cached data + // Check if we're offline - if so, try to use cached data (only if it's for the same user) const isOnline = navigator.onLine; if (!isOnline) { const cached = loadCachedProjectList(); - if (cached) { + if (cached?.projects && cached.userId === userId) { setStore('projectList', { - items: cached, + items: cached.projects, loaded: true, loading: false, error: null, + cachedUserId: userId, }); - return cached; + return cached.projects; } // No cached data available offline setStore('projectList', { @@ -430,25 +452,27 @@ function createProjectStore() { loaded: true, loading: false, error: null, + cachedUserId: userId, }); // Save to cache for offline access - saveCachedProjectList(projects); + saveCachedProjectList(projects, userId); return projects; } catch (err) { console.error('Error fetching projects:', err); - // If fetch failed, try to use cached data as fallback + // If fetch failed, try to use cached data as fallback (only if it's for the same user) const cached = loadCachedProjectList(); - if (cached) { + if (cached?.projects && cached.userId === userId) { setStore('projectList', { - items: cached, + items: cached.projects, loaded: true, loading: false, error: 'Using cached data - connection error', + cachedUserId: userId, }); - return cached; + return cached.projects; } setStore('projectList', { From c3cdeb4ef053fc5e0f09863bbd92ed5c8f05070a Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Fri, 19 Dec 2025 20:51:04 +0000 Subject: [PATCH 2/7] Apply Prettier formatting --- eslint.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index c216584c1..a3f4ea182 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -41,7 +41,7 @@ export default [ document: 'readonly', navigator: 'readonly', console: 'readonly', - + fetch: 'readonly', URL: 'readonly', caches: 'readonly', From 674b7def677de996c8a27d95419bbc493865950d Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Fri, 19 Dec 2025 15:48:12 -0600 Subject: [PATCH 3/7] fix navbar ux, final answers not working, refactored state management --- .../checklist-ui/compare/AnswerPanel.jsx | 6 +- .../compare/ChecklistReconciliation.jsx | 259 +++++++++++------- .../checklist-ui/compare/Navbar.jsx | 13 +- .../compare/ReconciliationQuestionPage.jsx | 4 +- .../compare/ReconciliationWithPdf.jsx | 18 +- .../compare/ReconciliationWrapper.jsx | 211 +++++++++++--- .../checklist-ui/compare/navbar-utils.js | 10 +- .../web/src/primitives/useProject/index.js | 2 - .../primitives/useProject/reconciliation.js | 123 +-------- 9 files changed, 364 insertions(+), 282 deletions(-) diff --git a/packages/web/src/components/checklist-ui/compare/AnswerPanel.jsx b/packages/web/src/components/checklist-ui/compare/AnswerPanel.jsx index 6faacf5a8..0b20bbec4 100644 --- a/packages/web/src/components/checklist-ui/compare/AnswerPanel.jsx +++ b/packages/web/src/components/checklist-ui/compare/AnswerPanel.jsx @@ -98,9 +98,9 @@ export default function AnswerPanel(props) { >
{/* Panel Header */} -
+
-

{props.title}

+

{props.title}

{props.selectedSource === 'custom' ? @@ -127,7 +127,7 @@ export default function AnswerPanel(props) {
Result: {props.finalAnswer || 'Not selected'} diff --git a/packages/web/src/components/checklist-ui/compare/ChecklistReconciliation.jsx b/packages/web/src/components/checklist-ui/compare/ChecklistReconciliation.jsx index 959d36da3..3b146d3db 100644 --- a/packages/web/src/components/checklist-ui/compare/ChecklistReconciliation.jsx +++ b/packages/web/src/components/checklist-ui/compare/ChecklistReconciliation.jsx @@ -19,23 +19,15 @@ import SummaryView from './SummaryView.jsx'; export default function ChecklistReconciliation(props) { // props.checklist1 - First reviewer's checklist data // props.checklist2 - Second reviewer's checklist data - // props.onSaveReconciled - Callback when reconciled checklist is saved - // props.onSaveProgress - Callback to save progress for resuming later - // props.savedProgress - Previously saved progress (if resuming) + // props.reconciledChecklist - The reconciled checklist data (read from Yjs, reactive) + // props.reconciledChecklistId - ID of the reconciled checklist + // props.onSaveReconciled - Callback when reconciled checklist is saved (receives reconciledName) // props.onCancel - Callback to cancel and go back // props.reviewer1Name - Display name for first reviewer // props.reviewer2Name - Display name for second reviewer // props.setNavbarStore - Store setter for navbar state (deep reactivity) - // props.getReconciliationNote - Function to get Y.Text for final reconciliation note (questionKey => Y.Text) - - // Track if we've initialized from saved progress - const [initialized, setInitialized] = createSignal(false); - - // Current page index - const [currentPage, setCurrentPage] = createSignal(0); - - // Final answers for each question (the reconciled/merged answers) - const [finalAnswers, setFinalAnswers] = createSignal({}); + // props.getQuestionNote - Function to get Y.Text for a question note (questionKey => Y.Text) + // props.updateChecklistAnswer - Function to update a question answer in the reconciled checklist // Reconciled checklist metadata const [reconciledName, setReconciledName] = createSignal('Reconciled Checklist'); @@ -43,9 +35,54 @@ export default function ChecklistReconciliation(props) { const confirmDialog = useConfirmDialog(); - // View mode: 'questions' or 'summary' + // Navigation state in localStorage (not synced across clients) + const getStorageKey = () => { + if (!props.checklist1?.id || !props.checklist2?.id) return null; + return `reconciliation-nav-${props.checklist1.id}-${props.checklist2.id}`; + }; + + // Save navigation state to localStorage + const saveNavigationState = (page, mode) => { + const key = getStorageKey(); + if (!key) return; + try { + localStorage.setItem(key, JSON.stringify({ currentPage: page, viewMode: mode })); + } catch (e) { + console.error('Failed to save navigation state:', e); + } + }; + + // Load initial navigation state from localStorage (in tracked scope) + const [currentPage, setCurrentPage] = createSignal(0); const [viewMode, setViewMode] = createSignal('questions'); + // Initialize navigation state from localStorage + createEffect(() => { + // Only run once when checklists are available + if (!props.checklist1?.id || !props.checklist2?.id) return; + + const key = getStorageKey(); + if (!key) return; + + try { + const stored = localStorage.getItem(key); + if (stored) { + const parsed = JSON.parse(stored); + setCurrentPage(parsed.currentPage ?? 0); + setViewMode(parsed.viewMode ?? 'questions'); + } + } catch (e) { + console.error('Failed to load navigation state:', e); + } + }); + + // Save navigation state when it changes + createEffect(() => { + const page = currentPage(); + const mode = viewMode(); + saveNavigationState(page, mode); + }); + // All question keys in order const questionKeys = getQuestionKeys(); const totalPages = questionKeys.length; @@ -75,6 +112,38 @@ export default function ChecklistReconciliation(props) { return map; }); + // Get finalAnswers from reconciled checklist (reactive to Yjs changes) + const finalAnswers = createMemo(() => { + const reconciled = props.reconciledChecklist; + if (!reconciled) return {}; + // Extract question answers (q1, q2, etc.) from reconciled checklist + // Handle multi-part questions (q9, q11) by nesting q9a/q9b under q9 + const answers = {}; + for (const key of questionKeys) { + if (isMultiPartQuestion(key)) { + // For multi-part questions, nest the parts under the parent key + const dataKeys = getDataKeysForQuestion(key); + const parts = {}; + let hasAnyPart = false; + for (const dk of dataKeys) { + if (reconciled[dk]) { + parts[dk] = reconciled[dk]; + hasAnyPart = true; + } + } + if (hasAnyPart) { + answers[key] = parts; + } + } else { + // Regular question + if (reconciled[key]) { + answers[key] = reconciled[key]; + } + } + } + return answers; + }); + // Expose navbar props for external rendering via store createEffect(() => { if (props.setNavbarStore) { @@ -126,59 +195,50 @@ export default function ChecklistReconciliation(props) { return ''; }; - // Initialize final answers from saved progress or auto-fill agreements - createEffect(() => { - if (!props.checklist1 || initialized()) return; - - // If we have saved progress with actual answers, use it - if ( - props.savedProgress?.finalAnswers && - Object.keys(props.savedProgress.finalAnswers).length > 0 - ) { - setFinalAnswers(props.savedProgress.finalAnswers); - setCurrentPage(props.savedProgress.currentPage || 0); - setViewMode(props.savedProgress.viewMode || 'questions'); - setInitialized(true); - return; - } + // No initialization needed - finalAnswers come from reconciled checklist (reactive to Yjs) - // Initialize with empty answers - all questions require manual reconciliation - const comp = comparison(); - if (!comp) { - setInitialized(true); - return; - } - - // Start with empty final answers - user must manually reconcile everything - setFinalAnswers({}); - setInitialized(true); - }); + // Get current question's final answer from reconciled checklist + const currentFinalAnswer = () => { + const reconciled = props.reconciledChecklist; + if (!reconciled) return null; + const key = currentQuestionKey(); - // Save progress whenever state changes (debounced via effect) - createEffect(() => { - // Skip if not initialized or no save function - if (!initialized() || !props.onSaveProgress) return; - - // Capture current state - const progress = { - currentPage: currentPage(), - viewMode: viewMode(), - finalAnswers: finalAnswers(), - }; - - // Save progress - props.onSaveProgress(progress); - }); + // Handle multi-part questions (q9, q11) - data is stored as q9a/q9b + if (isMultiPartQuestion(key)) { + const dataKeys = getDataKeysForQuestion(key); + const parts = {}; + let hasAnyPart = false; + for (const dk of dataKeys) { + if (reconciled[dk]) { + parts[dk] = reconciled[dk]; + hasAnyPart = true; + } + } + return hasAnyPart ? parts : null; + } - // Get current question's final answer - const currentFinalAnswer = () => finalAnswers()[currentQuestionKey()]; + // Regular question + return reconciled[key] || null; + }; - // Update final answer for current question + // Update final answer for current question - write directly to Yjs via updateChecklistAnswer function handleFinalChange(newAnswer) { - setFinalAnswers(prev => ({ - ...prev, - [currentQuestionKey()]: newAnswer, - })); + if (!props.updateChecklistAnswer) return; + const key = currentQuestionKey(); + + // Handle multi-part questions (q9, q11) - newAnswer is { q9a: {...}, q9b: {...} } + if (isMultiPartQuestion(key)) { + const dataKeys = getDataKeysForQuestion(key); + for (const dk of dataKeys) { + if (newAnswer[dk]) { + props.updateChecklistAnswer(dk, newAnswer[dk]); + } + } + } else { + // Regular question - newAnswer is the question data directly + props.updateChecklistAnswer(key, newAnswer); + } + // Note: No need to update local state - Yjs will sync and reconciledChecklist will update reactively } // Navigation @@ -188,13 +248,12 @@ export default function ChecklistReconciliation(props) { const currentFinal = finalAnswers()[key]; // If no final answer set yet, use reviewer1's answer as default - if (!currentFinal && props.checklist1) { + if (!currentFinal && props.checklist1 && props.updateChecklistAnswer) { const defaultAnswer = getReviewerAnswers(props.checklist1, key); if (defaultAnswer) { - setFinalAnswers(prev => ({ - ...prev, - [key]: JSON.parse(JSON.stringify(defaultAnswer)), - })); + // Write directly to Yjs via updateChecklistAnswer + // handleFinalChange will handle multi-part questions correctly + handleFinalChange(JSON.parse(JSON.stringify(defaultAnswer))); } } @@ -222,9 +281,33 @@ export default function ChecklistReconciliation(props) { } // Reset all reconciliation answers - function handleReset() { - // Clear all answers - everything goes back to unresolved - setFinalAnswers({}); + async function handleReset() { + if (!props.updateChecklistAnswer) return; + + // Get default empty structure from createChecklist + const { createChecklist } = await import('@/AMSTAR2/checklist.js'); + const defaultChecklist = createChecklist({ + name: 'temp', + id: 'temp', + }); + + // Clear all answers in the reconciled checklist by setting to default empty structure + for (const key of questionKeys) { + if (isMultiPartQuestion(key)) { + // For multi-part questions, reset each part + const dataKeys = getDataKeysForQuestion(key); + for (const dk of dataKeys) { + if (defaultChecklist[dk]) { + props.updateChecklistAnswer(dk, defaultChecklist[dk]); + } + } + } else { + if (defaultChecklist[key]) { + props.updateChecklistAnswer(key, defaultChecklist[key]); + } + } + } + setCurrentPage(0); setViewMode('questions'); showToast.info('Reconciliation Reset', 'All reconciliations have been cleared.'); @@ -279,7 +362,7 @@ export default function ChecklistReconciliation(props) { const confirmed = await confirmDialog.open({ title: 'Finish reconciliation?', description: - 'This will save a final reconciled checklist and end this reconciliation. You will no longer be able to edit these reconciliation answers afterwards.', + 'This will mark the reconciled checklist as completed and end this reconciliation. You will no longer be able to edit these reconciliation answers afterwards.', confirmText: 'Finish', cancelText: 'Cancel', variant: 'warning', @@ -289,36 +372,8 @@ export default function ChecklistReconciliation(props) { setSaving(true); try { - // Build the reconciled checklist object - flatten multi-part questions - const finals = finalAnswers(); - const flattenedAnswers = {}; - for (const key of questionKeys) { - if (isMultiPartQuestion(key)) { - // Flatten q9 -> q9a, q9b or q11 -> q11a, q11b - const dataKeys = getDataKeysForQuestion(key); - for (const dk of dataKeys) { - if (finals[key]?.[dk]) { - flattenedAnswers[dk] = finals[key][dk]; - } - } - } else { - if (finals[key]) { - flattenedAnswers[key] = finals[key]; - } - } - } - - const reconciled = { - name: reconciledName(), - reviewerName: 'Consensus', - createdAt: new Date().toISOString().split('T')[0], - id: `reconciled-${Date.now()}`, - isReconciled: true, - sourceChecklists: [props.checklist1?.id, props.checklist2?.id], - ...flattenedAnswers, - }; - - await props.onSaveReconciled?.(reconciled); + // Just pass the name - the checklist already exists and has all the answers + await props.onSaveReconciled?.(reconciledName()); } catch (err) { console.error('Error saving reconciled checklist:', err); showToast.error('Save Failed', 'Failed to save reconciled checklist. Please try again.'); @@ -349,7 +404,7 @@ export default function ChecklistReconciliation(props) { isMultiPart={isMultiPartQuestion(currentQuestionKey())} reviewer1Note={getReviewerNote(props.checklist1, currentQuestionKey())} reviewer2Note={getReviewerNote(props.checklist2, currentQuestionKey())} - finalNoteYText={props.getReconciliationNote?.(currentQuestionKey())} + finalNoteYText={props.getQuestionNote?.(currentQuestionKey())} /> {/* Navigation Buttons */} diff --git a/packages/web/src/components/checklist-ui/compare/Navbar.jsx b/packages/web/src/components/checklist-ui/compare/Navbar.jsx index 246ab6ec4..c3c64c125 100644 --- a/packages/web/src/components/checklist-ui/compare/Navbar.jsx +++ b/packages/web/src/components/checklist-ui/compare/Navbar.jsx @@ -1,5 +1,6 @@ -import { For, createMemo } from 'solid-js'; +import { For, createMemo, Show } from 'solid-js'; import { FaSolidArrowRotateLeft } from 'solid-icons/fa'; +import { FiCheck } from 'solid-icons/fi'; import { Tooltip } from '@corates/ui'; import { hasQuestionAnswer, getQuestionPillStyle, getQuestionTooltip } from './navbar-utils.js'; @@ -56,11 +57,19 @@ function QuestionPill(props) { ); diff --git a/packages/web/src/components/checklist-ui/compare/ReconciliationQuestionPage.jsx b/packages/web/src/components/checklist-ui/compare/ReconciliationQuestionPage.jsx index 63743ad66..42f8e7657 100644 --- a/packages/web/src/components/checklist-ui/compare/ReconciliationQuestionPage.jsx +++ b/packages/web/src/components/checklist-ui/compare/ReconciliationQuestionPage.jsx @@ -152,7 +152,7 @@ function SingleQuestionPage(props) {
-

+

{question()?.text} (Critical) @@ -160,7 +160,7 @@ function SingleQuestionPage(props) {

{props.isAgreement ? 'Reviewers Agree' : 'Requires Reconciliation'} diff --git a/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx b/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx index 871d29921..cb4af0739 100644 --- a/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx +++ b/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx @@ -14,9 +14,9 @@ import SplitPanelControls from '@/components/checklist-ui/SplitPanelControls.jsx export default function ReconciliationWithPdf(props) { // props.checklist1 - First reviewer's checklist data // props.checklist2 - Second reviewer's checklist data - // props.onSaveReconciled - Callback when reconciled checklist is saved - // props.onSaveProgress - Callback to save progress for resuming later - // props.savedProgress - Previously saved progress (if resuming) + // props.reconciledChecklist - The reconciled checklist data (third checklist both reviewers edit) + // props.reconciledChecklistId - ID of the reconciled checklist + // props.onSaveReconciled - Callback when reconciled checklist is saved (receives reconciledName) // props.onCancel - Callback to cancel and go back // props.reviewer1Name - Display name for first reviewer // props.reviewer2Name - Display name for second reviewer @@ -26,7 +26,8 @@ export default function ReconciliationWithPdf(props) { // props.pdfs - Array of PDFs for multi-PDF selection // props.selectedPdfId - Currently selected PDF ID // props.onPdfSelect - Handler for PDF selection change - // props.getReconciliationNote - Function to get Y.Text for a reconciliation note (questionKey => Y.Text) + // props.getQuestionNote - Function to get Y.Text for a question note (questionKey => Y.Text) + // props.updateChecklistAnswer - Function to update a question answer (questionKey, questionData) // Layout state const [showPdf, setShowPdf] = createSignal(true); @@ -159,14 +160,17 @@ export default function ReconciliationWithPdf(props) { + props.updateChecklistAnswer?.(questionKey, questionData) + } />
diff --git a/packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx b/packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx index 91ddbfad8..d91161001 100644 --- a/packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx +++ b/packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx @@ -28,7 +28,7 @@ export default function ReconciliationWrapper() { updateChecklist, getChecklistData, getReconciliationProgress, - getReconciliationNote, + getQuestionNote, saveReconciliationProgress, clearReconciliationProgress, connect, @@ -197,64 +197,172 @@ export default function ReconciliationWrapper() { return result; }); - // Get saved reconciliation progress - const savedProgress = createMemo(() => { - // Make sure we're synced before trying to read progress - if (!connectionState().synced) return null; + // State for reconciled checklist + const [reconciledChecklistId, setReconciledChecklistId] = createSignal(null); + const [reconciledChecklistLoading, setReconciledChecklistLoading] = createSignal(false); + const [hasCheckedForReconciled, setHasCheckedForReconciled] = createSignal(false); - // Only return progress if it matches the current checklists being reconciled + // Get or create reconciled checklist (with race condition prevention) + createEffect(() => { + const state = connectionState(); + if (!state.synced || reconciledChecklistId() || hasCheckedForReconciled()) return; + + setHasCheckedForReconciled(true); + setReconciledChecklistLoading(true); + + const study = currentStudy(); + if (!study) { + setReconciledChecklistLoading(false); + setHasCheckedForReconciled(false); // Allow retry + return; + } + + // First, check if one already exists in reconciliation progress const progress = getReconciliationProgress(params.studyId); - if (!progress) return null; if ( + progress && progress.checklist1Id === params.checklist1Id && - progress.checklist2Id === params.checklist2Id + progress.checklist2Id === params.checklist2Id && + progress.reconciledChecklistId ) { - return progress; + // Verify it still exists in study checklists + const existingChecklist = study.checklists?.find( + c => c.id === progress.reconciledChecklistId && c.isReconciled, + ); + if (existingChecklist) { + setReconciledChecklistId(progress.reconciledChecklistId); + setReconciledChecklistLoading(false); + return; + } } - return null; - }); - // Get reviewer name from userId - function getReviewerName(userId) { - if (!userId) return 'Unassigned'; - const member = members().find(m => m.userId === userId); - return member?.displayName || member?.name || member?.email || 'Unknown'; - } + // Check if one exists in study checklists (another client may have created it) + const existingReconciled = study.checklists?.find( + c => c.isReconciled && c.status !== 'completed', + ); + if (existingReconciled) { + // Found existing - save reference in progress and use it + saveReconciliationProgress(params.studyId, { + checklist1Id: params.checklist1Id, + checklist2Id: params.checklist2Id, + reconciledChecklistId: existingReconciled.id, + }); + setReconciledChecklistId(existingReconciled.id); + setReconciledChecklistLoading(false); + return; + } + + // Need to create one - detect checklist type from checklist1 + const checklist1 = checklist1Meta(); + const checklistType = checklist1?.type || 'AMSTAR2'; + + // Create the reconciled checklist + const newChecklistId = createProjectChecklist(params.studyId, checklistType, null); + if (!newChecklistId) { + setError('Failed to create reconciled checklist'); + setReconciledChecklistLoading(false); + return; + } + + // Mark it as reconciled and in_progress + updateChecklist(params.studyId, newChecklistId, { + isReconciled: true, + status: 'in_progress', + title: 'Reconciled Checklist', + }); - // Handle saving reconciliation progress - function handleSaveProgress(progressData) { + // Save reference in reconciliation progress saveReconciliationProgress(params.studyId, { checklist1Id: params.checklist1Id, checklist2Id: params.checklist2Id, - currentPage: progressData.currentPage, - viewMode: progressData.viewMode, - finalAnswers: progressData.finalAnswers, + reconciledChecklistId: newChecklistId, }); - } - // Handle saving the reconciled checklist - async function handleSaveReconciled(reconciledChecklist) { - try { - // Create a new checklist in the study - const newChecklistId = createProjectChecklist(params.studyId, 'AMSTAR2', null); + // Set the ID - if another client created one, the store will update reactively + // and we'll see it in the next effect run + setReconciledChecklistId(newChecklistId); + setReconciledChecklistLoading(false); + }); - if (!newChecklistId) { - throw new Error('Failed to create reconciled checklist'); + // Watch for race condition: if another client created a reconciled checklist, + // use the one created first (check happens reactively via store updates) + createEffect(() => { + if (!connectionState().synced || !reconciledChecklistId() || reconciledChecklistLoading()) + return; + + const study = currentStudy(); + const currentId = reconciledChecklistId(); + if (!study || !currentId) return; + + // Check if there's another reconciled checklist (another client may have created one) + const allReconciled = + study.checklists?.filter(c => c.isReconciled && c.status !== 'completed') || []; + + if (allReconciled.length > 1) { + // Multiple reconciled checklists exist - use the one created first + allReconciled.sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); + const firstCreated = allReconciled[0]; + + if (firstCreated.id !== currentId) { + // Another one was created first - use it instead + saveReconciliationProgress(params.studyId, { + checklist1Id: params.checklist1Id, + checklist2Id: params.checklist2Id, + reconciledChecklistId: firstCreated.id, + }); + setReconciledChecklistId(firstCreated.id); } + } + }); - // Update each question's answer - const questionKeys = Object.keys(reconciledChecklist).filter(k => /^q\d+[a-z]*$/i.test(k)); + // Get reconciled checklist metadata + const reconciledChecklistMeta = createMemo(() => { + const study = currentStudy(); + const id = reconciledChecklistId(); + if (!study || !id) return null; + return study.checklists?.find(c => c.id === id); + }); - for (const key of questionKeys) { - updateChecklistAnswer(params.studyId, newChecklistId, key, reconciledChecklist[key]); + // Get reconciled checklist data + const reconciledChecklistData = createMemo(() => { + const id = reconciledChecklistId(); + if (!id) return null; + + const data = getChecklistData(params.studyId, id); + if (!data) return null; + + // Flatten answers into the checklist object (expected format) + const result = { + id, + name: 'Reconciled Checklist', + reviewerName: 'Consensus', + createdAt: reconciledChecklistMeta()?.createdAt || Date.now(), + ...data.answers, + }; + + return result; + }); + + // Get reviewer name from userId + function getReviewerName(userId) { + if (!userId) return 'Unassigned'; + const member = members().find(m => m.userId === userId); + return member?.displayName || member?.name || member?.email || 'Unknown'; + } + + // Handle saving the reconciled checklist + async function handleSaveReconciled(reconciledName) { + try { + const id = reconciledChecklistId(); + if (!id) { + throw new Error('No reconciled checklist found'); } - // Mark it as a reconciled/consensus checklist so it appears in the Completed tab - updateChecklist(params.studyId, newChecklistId, { + // Mark the reconciled checklist as completed + updateChecklist(params.studyId, id, { status: 'completed', - title: 'Reconciled Checklist', + title: reconciledName || 'Reconciled Checklist', isReconciled: true, - assignedTo: null, }); // Clear the reconciliation progress since we've completed it @@ -299,12 +407,22 @@ export default function ReconciliationWrapper() { } >
-

Loading checklists...

+

+ {reconciledChecklistLoading() ? + 'Setting up reconciliation...' + : 'Loading checklists...'} +

} @@ -312,10 +430,10 @@ export default function ReconciliationWrapper() { getReconciliationNote(params.studyId, questionKey)} + getQuestionNote={questionKey => + getQuestionNote(params.studyId, reconciledChecklistId(), questionKey) + } + updateChecklistAnswer={(questionKey, questionData) => { + const id = reconciledChecklistId(); + if (!id) return; + updateChecklistAnswer(params.studyId, id, questionKey, questionData); + }} /> diff --git a/packages/web/src/components/checklist-ui/compare/navbar-utils.js b/packages/web/src/components/checklist-ui/compare/navbar-utils.js index bad693d77..b613132ab 100644 --- a/packages/web/src/components/checklist-ui/compare/navbar-utils.js +++ b/packages/web/src/components/checklist-ui/compare/navbar-utils.js @@ -32,7 +32,7 @@ export function hasQuestionAnswer(questionKey, finalAnswers) { /** * Get pill styling classes based on question state * @param {boolean} isCurrentPage - Is this the active page - * @param {boolean} hasAnswer - Has this question been answered + * @param {boolean} hasAnswer - Has this question been answered (not used for styling, checkmark overlay handles it) * @param {boolean} isAgreement - Do reviewers agree on this question * @returns {string} Tailwind CSS classes */ @@ -40,13 +40,7 @@ export function getQuestionPillStyle(isCurrentPage, hasAnswer, isAgreement) { if (isCurrentPage) { return 'bg-blue-600 text-white ring-2 ring-blue-300'; } - if (hasAnswer) { - // Answered - show solid color - return isAgreement ? - 'bg-green-500 text-white hover:bg-green-600' - : 'bg-amber-500 text-white hover:bg-amber-600'; - } - // Not answered yet - show lighter color to indicate agreement/disagreement status + // Always use lighter colors - checkmark icon indicates if answered return isAgreement ? 'bg-green-100 text-green-700 hover:bg-green-200' : 'bg-amber-100 text-amber-700 hover:bg-amber-200'; diff --git a/packages/web/src/primitives/useProject/index.js b/packages/web/src/primitives/useProject/index.js index 8d722f3b5..61b4bdd5e 100644 --- a/packages/web/src/primitives/useProject/index.js +++ b/packages/web/src/primitives/useProject/index.js @@ -350,8 +350,6 @@ export function useProject(projectId) { connectionEntry?.reconciliationOps?.saveReconciliationProgress(...args), getReconciliationProgress: (...args) => connectionEntry?.reconciliationOps?.getReconciliationProgress(...args), - getReconciliationNote: (...args) => - connectionEntry?.reconciliationOps?.getReconciliationNote(...args), clearReconciliationProgress: (...args) => connectionEntry?.reconciliationOps?.clearReconciliationProgress(...args), diff --git a/packages/web/src/primitives/useProject/reconciliation.js b/packages/web/src/primitives/useProject/reconciliation.js index ce504670f..4a360a455 100644 --- a/packages/web/src/primitives/useProject/reconciliation.js +++ b/packages/web/src/primitives/useProject/reconciliation.js @@ -1,8 +1,9 @@ /** * Reconciliation progress operations for useProject * - * Note: finalAnswers are stored as a nested Y.Map structure to enable - * collaborative editing of notes during reconciliation. + * Note: finalAnswers are now stored in a third checklist (reconciled checklist) + * that both reviewers can edit. This leverages existing checklist infrastructure + * for automatic Yjs sync. Reconciliation progress only stores metadata references. */ import * as Y from 'yjs'; @@ -17,9 +18,9 @@ import * as Y from 'yjs'; export function createReconciliationOperations(projectId, getYDoc, isSynced) { /** * Save reconciliation progress for a study - * Uses Y.Map for finalAnswers to enable collaborative note editing + * Stores only metadata references - finalAnswers are in the reconciled checklist * @param {string} studyId - The study ID - * @param {Object} progressData - Progress data { checklist1Id, checklist2Id, currentPage, viewMode, finalAnswers } + * @param {Object} progressData - Progress data { checklist1Id, checklist2Id, reconciledChecklistId } */ function saveReconciliationProgress(studyId, progressData) { const ydoc = getYDoc(); @@ -36,42 +37,13 @@ export function createReconciliationOperations(projectId, getYDoc, isSynced) { studyYMap.set('reconciliation', reconciliationMap); } - // Save the progress data + // Save the progress data (minimal - just references) reconciliationMap.set('checklist1Id', progressData.checklist1Id); reconciliationMap.set('checklist2Id', progressData.checklist2Id); - reconciliationMap.set('currentPage', progressData.currentPage); - reconciliationMap.set('viewMode', progressData.viewMode || 'questions'); - reconciliationMap.set('updatedAt', Date.now()); - - // Store finalAnswers as a nested Y.Map for collaborative editing - let finalAnswersMap = reconciliationMap.get('finalAnswers'); - if (!finalAnswersMap || !(finalAnswersMap instanceof Y.Map)) { - finalAnswersMap = new Y.Map(); - reconciliationMap.set('finalAnswers', finalAnswersMap); + if (progressData.reconciledChecklistId) { + reconciliationMap.set('reconciledChecklistId', progressData.reconciledChecklistId); } - - // Update each question's answers - const finalAnswers = progressData.finalAnswers || {}; - Object.entries(finalAnswers).forEach(([questionKey, questionData]) => { - let questionYMap = finalAnswersMap.get(questionKey); - if (!questionYMap || !(questionYMap instanceof Y.Map)) { - questionYMap = new Y.Map(); - finalAnswersMap.set(questionKey, questionYMap); - } - - // Store answers and critical flag - if (questionData.answers !== undefined) { - questionYMap.set('answers', questionData.answers); - } - if (questionData.critical !== undefined) { - questionYMap.set('critical', questionData.critical); - } - - // Initialize Y.Text for note if not present (for collaborative editing) - if (!questionYMap.get('note')) { - questionYMap.set('note', new Y.Text()); - } - }); + reconciliationMap.set('updatedAt', Date.now()); studyYMap.set('updatedAt', Date.now()); } @@ -96,88 +68,14 @@ export function createReconciliationOperations(projectId, getYDoc, isSynced) { const checklist2Id = reconciliationMap.get('checklist2Id'); if (!checklist1Id || !checklist2Id) return null; - // Extract finalAnswers from Y.Map structure - const finalAnswers = {}; - const finalAnswersMap = reconciliationMap.get('finalAnswers'); - if (finalAnswersMap && finalAnswersMap instanceof Y.Map) { - for (const [questionKey, questionYMap] of finalAnswersMap.entries()) { - if (questionYMap instanceof Y.Map) { - finalAnswers[questionKey] = { - answers: questionYMap.get('answers'), - critical: questionYMap.get('critical'), - // Note: Y.Text note is accessed separately via getReconciliationNote - }; - } - } - } - return { checklist1Id, checklist2Id, - currentPage: reconciliationMap.get('currentPage') || 0, - viewMode: reconciliationMap.get('viewMode') || 'questions', - finalAnswers, + reconciledChecklistId: reconciliationMap.get('reconciledChecklistId') || null, updatedAt: reconciliationMap.get('updatedAt'), }; } - /** - * Get a Y.Text reference for a reconciliation note (for direct binding) - * @param {string} studyId - The study ID - * @param {string} questionKey - The question key (e.g., 'q1', 'q9') - * @returns {Y.Text|null} The Y.Text reference or null - */ - function getReconciliationNote(studyId, questionKey) { - const ydoc = getYDoc(); - if (!ydoc) return null; - - const studiesMap = ydoc.getMap('reviews'); - const studyYMap = studiesMap.get(studyId); - if (!studyYMap) return null; - - // Create reconciliation map if it doesn't exist - let reconciliationMap = studyYMap.get('reconciliation'); - if (!reconciliationMap) { - if (!isSynced()) return null; - reconciliationMap = new Y.Map(); - studyYMap.set('reconciliation', reconciliationMap); - } - - // Create finalAnswers map if it doesn't exist - let finalAnswersMap = reconciliationMap.get('finalAnswers'); - if (!finalAnswersMap || !(finalAnswersMap instanceof Y.Map)) { - if (!isSynced()) return null; - finalAnswersMap = new Y.Map(); - reconciliationMap.set('finalAnswers', finalAnswersMap); - } - - let questionYMap = finalAnswersMap.get(questionKey); - if (!questionYMap || !(questionYMap instanceof Y.Map)) { - // Create the question map if it doesn't exist - if (isSynced()) { - questionYMap = new Y.Map(); - questionYMap.set('note', new Y.Text()); - finalAnswersMap.set(questionKey, questionYMap); - return questionYMap.get('note'); - } - return null; - } - - let note = questionYMap.get('note'); - if (note instanceof Y.Text) { - return note; - } - - // Create note if missing - if (isSynced()) { - note = new Y.Text(); - questionYMap.set('note', note); - return note; - } - - return null; - } - /** * Clear reconciliation progress for a study * @param {string} studyId - The study ID @@ -197,7 +95,6 @@ export function createReconciliationOperations(projectId, getYDoc, isSynced) { return { saveReconciliationProgress, getReconciliationProgress, - getReconciliationNote, clearReconciliationProgress, }; } From b0b024062d19a2c21cdc7b2273ff6748e1d6c1df Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Fri, 19 Dec 2025 16:02:22 -0600 Subject: [PATCH 4/7] fix some bugs we introduced --- packages/ui/src/zag/Tooltip.jsx | 24 +++++++++++-------- .../src/components/project-ui/ProjectView.jsx | 15 +++++++++--- .../project-ui/reconcile-tab/ReconcileTab.jsx | 17 +++++++++---- 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/packages/ui/src/zag/Tooltip.jsx b/packages/ui/src/zag/Tooltip.jsx index b176b4caf..ab5c33b23 100644 --- a/packages/ui/src/zag/Tooltip.jsx +++ b/packages/ui/src/zag/Tooltip.jsx @@ -1,6 +1,7 @@ import * as tooltip from '@zag-js/tooltip'; import { normalizeProps, useMachine } from '@zag-js/solid'; import { createMemo, createUniqueId, Show } from 'solid-js'; +import { Portal } from 'solid-js/web'; export function Tooltip(props) { const service = useMachine(tooltip.machine, () => ({ @@ -8,6 +9,7 @@ export function Tooltip(props) { positioning: { placement: props.placement || 'top', gutter: 8, + strategy: 'fixed', // Use fixed positioning to avoid stacking context issues }, openDelay: props.openDelay ?? 100, closeDelay: props.closeDelay ?? 100, @@ -20,17 +22,19 @@ export function Tooltip(props) { <> {props.children} -
-
-
+ +
+
+
+
+
+ {props.content} +
-
- {props.content} -
-
+ ); diff --git a/packages/web/src/components/project-ui/ProjectView.jsx b/packages/web/src/components/project-ui/ProjectView.jsx index 1346ee54d..ac578f422 100644 --- a/packages/web/src/components/project-ui/ProjectView.jsx +++ b/packages/web/src/components/project-ui/ProjectView.jsx @@ -192,13 +192,22 @@ export default function ProjectView() { }; const getReconcileCount = () => { - // Count dual-reviewer studies with at least 1 completed checklist (not reconciled yet) + // Count dual-reviewer studies with at least 1 completed checklist (not completed reconciled yet) + // Includes in-progress reconciliations return studies().filter(study => { if (!study.reviewer1 || !study.reviewer2) return false; const checklists = study.checklists || []; - if (checklists.some(c => c.isReconciled)) return false; + // Skip if already has a completed reconciled checklist + if (checklists.some(c => c.isReconciled && c.status === 'completed')) return false; const completedChecklists = checklists.filter(c => c.status === 'completed'); - return completedChecklists.length >= 1 && completedChecklists.length <= 2; + // Show if 1 or 2 checklists are completed, OR if there's an in-progress reconciled checklist + const hasInProgressReconciliation = checklists.some( + c => c.isReconciled && c.status !== 'completed', + ); + return ( + hasInProgressReconciliation || + (completedChecklists.length >= 1 && completedChecklists.length <= 2) + ); }).length; }; diff --git a/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx b/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx index 51a3114c9..bf984cda8 100644 --- a/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx +++ b/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx @@ -21,7 +21,7 @@ export default function ReconcileTab() { // Filter studies that are in reconciliation workflow: // - Dual reviewer studies (has both reviewer1 and reviewer2) // - Has at least 1 completed checklist - // - Does not have a reconciled checklist yet + // - Does not have a completed reconciled checklist (in-progress reconciliations are shown) const studiesInReconciliation = createMemo(() => { return studies().filter(study => { // Only for dual-reviewer studies @@ -29,14 +29,21 @@ export default function ReconcileTab() { const checklists = study.checklists || []; - // Skip if already has a reconciled checklist - if (checklists.some(c => c.isReconciled)) return false; + // Skip if already has a completed reconciled checklist + // But show if there's an in-progress reconciliation + if (checklists.some(c => c.isReconciled && c.status === 'completed')) return false; // Count completed checklists const completedChecklists = checklists.filter(c => c.status === 'completed'); - // Show if 1 or 2 checklists are completed - return completedChecklists.length >= 1 && completedChecklists.length <= 2; + // Show if 1 or 2 checklists are completed, OR if there's an in-progress reconciled checklist + const hasInProgressReconciliation = checklists.some( + c => c.isReconciled && c.status !== 'completed', + ); + return ( + hasInProgressReconciliation || + (completedChecklists.length >= 1 && completedChecklists.length <= 2) + ); }); }); From f3faaf3e229a871ce3346de054dfbfb3071ddadc Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Fri, 19 Dec 2025 16:08:31 -0600 Subject: [PATCH 5/7] reuse splitscreenlayout for reconcile --- .../compare/ReconciliationWithPdf.jsx | 235 ++++++------------ 1 file changed, 76 insertions(+), 159 deletions(-) diff --git a/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx b/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx index cb4af0739..fccafb44c 100644 --- a/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx +++ b/packages/web/src/components/checklist-ui/compare/ReconciliationWithPdf.jsx @@ -3,13 +3,13 @@ * in a split-screen layout. The PDF is read-only during reconciliation. */ -import { createSignal, Show } from 'solid-js'; +import { Show, createMemo } from 'solid-js'; import { createStore } from 'solid-js/store'; import { AiOutlineArrowLeft } from 'solid-icons/ai'; import PdfViewer from '@/components/checklist-ui/pdf/PdfViewer.jsx'; import ChecklistReconciliation from './ChecklistReconciliation.jsx'; import Navbar from './Navbar.jsx'; -import SplitPanelControls from '@/components/checklist-ui/SplitPanelControls.jsx'; +import SplitScreenLayout from '@/components/checklist-ui/SplitScreenLayout.jsx'; export default function ReconciliationWithPdf(props) { // props.checklist1 - First reviewer's checklist data @@ -29,12 +29,6 @@ export default function ReconciliationWithPdf(props) { // props.getQuestionNote - Function to get Y.Text for a question note (questionKey => Y.Text) // props.updateChecklistAnswer - Function to update a question answer (questionKey, questionData) - // Layout state - const [showPdf, setShowPdf] = createSignal(true); - const [layout, setLayout] = createSignal('vertical'); // 'vertical' = side by side, 'horizontal' = stacked - const [splitRatio, setSplitRatio] = createSignal(60); // Slightly more space for reconciliation - const [isDragging, setIsDragging] = createSignal(false); - // Navbar store for deep reactivity - ChecklistReconciliation will update this const [navbarStore, setNavbarStore] = createStore({ questionKeys: [], @@ -50,167 +44,90 @@ export default function ReconciliationWithPdf(props) { onReset: null, }); - let containerRef; - - function handleMouseDown(e) { - e.preventDefault(); - setIsDragging(true); - - const handleMouseMove = event => { - if (!containerRef) return; - - const rect = containerRef.getBoundingClientRect(); - let ratio; - - if (layout() === 'vertical') { - ratio = ((event.clientX - rect.left) / rect.width) * 100; - } else { - ratio = ((event.clientY - rect.top) / rect.height) * 100; - } - - // Clamp between 30% and 80% - ratio = Math.max(30, Math.min(80, ratio)); - setSplitRatio(ratio); - }; - - const handleMouseUp = () => { - setIsDragging(false); - document.removeEventListener('mousemove', handleMouseMove); - document.removeEventListener('mouseup', handleMouseUp); - }; + // Check if we have PDF to show (reactive) + const hasPdf = createMemo(() => !!(props.pdfData || props.pdfLoading)); + + // Build header content with back button, title, and navbar + const headerContent = ( + <> + {/* Back button */} + + + {/* Title */} +
+

Reconciliation

+

+ {props.reviewer1Name || 'Reviewer 1'} vs {props.reviewer2Name || 'Reviewer 2'} +

+
- document.addEventListener('mousemove', handleMouseMove); - document.addEventListener('mouseup', handleMouseUp); - } +
- // Check if we have PDF to show - const hasPdf = () => props.pdfData || props.pdfLoading; + {/* Navbar - question navigation pills */} + 0}> +
+ +
+ + + ); return (
- {/* Header toolbar */} -
- {/* Back button */} - - - {/* Title */} -
-

Reconciliation

-

- {props.reviewer1Name || 'Reviewer 1'} vs {props.reviewer2Name || 'Reviewer 2'} -

-
- -
- - {/* Navbar - question navigation pills */} - 0}> -
- - {/* Progress indicator */} - {/*
- {navbarStore.reviewedCount}/ - {navbarStore.totalPages} - - - {navbarStore.summary.agreementCount} agree - - - {navbarStore.summary.disagreementCount} differ - - -
*/} -
- - - - setShowPdf(!showPdf())} - layout={layout()} - onSetLayout={setLayout} - onResetRatio={() => setSplitRatio(60)} - secondPanelLabel='PDF viewer' - defaultRatioLabel='60/40' - /> - -
- - {/* Split content area */} -
{/* First panel: Reconciliation view */} -
- - props.updateChecklistAnswer?.(questionKey, questionData) + + props.updateChecklistAnswer?.(questionKey, questionData) + } + /> + + {/* Second panel: PDF Viewer (read-only) - only rendered when PDF exists */} + + +
+
+ Loading PDF... +
+
} - /> -
- - {/* Divider / Resize handle */} - -
- - {/* Second panel: PDF Viewer (read-only) */} -
- -
-
- Loading PDF... -
-
- } - > - - -
+ + -
+
); } From eb55b8c3620af057c16ce15f426e366f6a1e0374 Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Fri, 19 Dec 2025 16:11:00 -0600 Subject: [PATCH 6/7] animate split screen layout --- .../checklist-ui/SplitScreenLayout.jsx | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/web/src/components/checklist-ui/SplitScreenLayout.jsx b/packages/web/src/components/checklist-ui/SplitScreenLayout.jsx index b577bc9a9..ba4d40a7c 100644 --- a/packages/web/src/components/checklist-ui/SplitScreenLayout.jsx +++ b/packages/web/src/components/checklist-ui/SplitScreenLayout.jsx @@ -104,7 +104,7 @@ export default function SplitScreenLayout(props) { > {/* First panel */}
- {/* Divider / Resize handle */} - + {/* Divider / Resize handle with animation */} +
+ - {/* Second panel */} + {/* Second panel with slide animation */} +
- {secondPanel()} +
{secondPanel()}
From bba6c3585d6de9d44ed24c6e49a10966ec7d3d71 Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Fri, 19 Dec 2025 16:15:43 -0600 Subject: [PATCH 7/7] refactor z-index usage in ui library, refactor reconcile count logic --- packages/ui/src/constants/zIndex.js | 44 +++++++++++++++++++ packages/ui/src/index.js | 3 ++ packages/ui/src/zag/Combobox.jsx | 3 +- packages/ui/src/zag/Dialog.jsx | 9 ++-- packages/ui/src/zag/Drawer.jsx | 8 +++- packages/ui/src/zag/Menu.jsx | 3 +- packages/ui/src/zag/Popover.jsx | 3 +- packages/ui/src/zag/Toast.jsx | 3 +- packages/ui/src/zag/Tooltip.jsx | 3 +- packages/ui/src/zag/Tour.jsx | 7 +-- .../admin-ui/ImpersonationBanner.jsx | 5 ++- .../components/charts/ChartSettingsModal.jsx | 5 ++- .../src/components/project-ui/ProjectView.jsx | 19 +------- .../project-ui/reconcile-tab/ReconcileTab.jsx | 29 ++---------- packages/web/src/utils/reconciliation.js | 39 ++++++++++++++++ 15 files changed, 123 insertions(+), 60 deletions(-) create mode 100644 packages/ui/src/constants/zIndex.js create mode 100644 packages/web/src/utils/reconciliation.js diff --git a/packages/ui/src/constants/zIndex.js b/packages/ui/src/constants/zIndex.js new file mode 100644 index 000000000..3d47b3ec1 --- /dev/null +++ b/packages/ui/src/constants/zIndex.js @@ -0,0 +1,44 @@ +/** + * Z-Index Constants + * + * Centralized z-index values for overlay components to ensure consistent layering. + * All values are Tailwind CSS classes for compatibility with existing setup. + * + * Z-Index Tier System: + * - z-40: Tooltips, Popovers, Menus, Comboboxes (lowest overlay tier, non-blocking) + * - z-50: Dialogs, Drawers, Toasts (modal overlays and notifications) + * - z-100: Tour backdrops, System banners (high-priority overlays) + * - z-101: Tour spotlight (above tour backdrop) + * - z-102: Tour content (highest, above all other overlays) + */ + +export const Z_INDEX = { + /** Tooltip and Popover - Lowest overlay tier for non-blocking hints */ + TOOLTIP: 'z-40', + POPOVER: 'z-40', + + /** Menu and Combobox - Dropdown menus and autocomplete */ + MENU: 'z-40', + COMBOBOX: 'z-40', + + /** Dialog and Drawer Backdrop - Modal backdrops */ + BACKDROP: 'z-50', + + /** Dialog and Drawer Content - Modal content */ + DIALOG: 'z-50', + + /** Toast - Toast notifications */ + TOAST: 'z-50', + + /** Tour Backdrop - Tour overlay backdrop */ + TOUR_BACKDROP: 'z-100', + + /** Tour Spotlight - Tour spotlight effect */ + TOUR_SPOTLIGHT: 'z-101', + + /** Tour Content - Tour content (highest overlay) */ + TOUR_CONTENT: 'z-102', + + /** Banner - System-level banners (e.g., ImpersonationBanner) */ + BANNER: 'z-100', +}; diff --git a/packages/ui/src/index.js b/packages/ui/src/index.js index 62851c338..08ae6f6c6 100644 --- a/packages/ui/src/index.js +++ b/packages/ui/src/index.js @@ -8,3 +8,6 @@ export { useWindowDrag } from './primitives/useWindowDrag.js'; // Re-export utilities export { cn } from './lib/cn.js'; + +// Re-export constants +export { Z_INDEX } from './constants/zIndex.js'; diff --git a/packages/ui/src/zag/Combobox.jsx b/packages/ui/src/zag/Combobox.jsx index b276afbbb..a6f04b2d2 100644 --- a/packages/ui/src/zag/Combobox.jsx +++ b/packages/ui/src/zag/Combobox.jsx @@ -3,6 +3,7 @@ import { Portal } from 'solid-js/web'; import { normalizeProps, useMachine } from '@zag-js/solid'; import { createMemo, createSignal, createUniqueId, For, Show, splitProps } from 'solid-js'; import { FiChevronDown, FiX, FiCheck } from 'solid-icons/fi'; +import { Z_INDEX } from '../constants/zIndex.js'; /** * Combobox - Searchable select with autocomplete @@ -75,7 +76,7 @@ export function Combobox(props) { 0}>
    {item => ( diff --git a/packages/ui/src/zag/Dialog.jsx b/packages/ui/src/zag/Dialog.jsx index f0d7acf23..de4235979 100644 --- a/packages/ui/src/zag/Dialog.jsx +++ b/packages/ui/src/zag/Dialog.jsx @@ -3,6 +3,7 @@ import { Portal } from 'solid-js/web'; import { useMachine, normalizeProps } from '@zag-js/solid'; import { createMemo, createSignal, createUniqueId, Show } from 'solid-js'; import { FiAlertTriangle, FiX } from 'solid-icons/fi'; +import { Z_INDEX } from '../constants/zIndex.js'; /** * Dialog - A generic dialog/modal component @@ -51,12 +52,12 @@ export function Dialog(props) { {/* Backdrop */}
    {/* Positioner - scrollable container */}
    {/* Content */}
    {/* Positioner */}
    {/* Content */}
    {/* Positioner - full height, aligned to side */} -
    +
    {/* Content - slides in from side */}
      {item => ( diff --git a/packages/ui/src/zag/Popover.jsx b/packages/ui/src/zag/Popover.jsx index dc8ce3ccf..c5e7d38d3 100644 --- a/packages/ui/src/zag/Popover.jsx +++ b/packages/ui/src/zag/Popover.jsx @@ -3,6 +3,7 @@ import { Portal } from 'solid-js/web'; import { normalizeProps, useMachine } from '@zag-js/solid'; import { createMemo, createUniqueId, Show, splitProps, mergeProps } from 'solid-js'; import { FiX } from 'solid-icons/fi'; +import { Z_INDEX } from '../constants/zIndex.js'; /** * Popover - Non-modal floating dialog @@ -52,7 +53,7 @@ export function Popover(props) {
      diff --git a/packages/ui/src/zag/Toast.jsx b/packages/ui/src/zag/Toast.jsx index 234ae2d68..86a411a4c 100644 --- a/packages/ui/src/zag/Toast.jsx +++ b/packages/ui/src/zag/Toast.jsx @@ -2,6 +2,7 @@ import * as toast from '@zag-js/toast'; import { normalizeProps, useMachine, Key } from '@zag-js/solid'; import { createMemo, createUniqueId, Show } from 'solid-js'; import { FiX, FiCheck, FiAlertCircle, FiInfo, FiLoader } from 'solid-icons/fi'; +import { Z_INDEX } from '../constants/zIndex.js'; /** * Create the toast store - this is the global toaster instance @@ -116,7 +117,7 @@ export function Toaster() { return (
      t.id}> {(toastItem, index) => } diff --git a/packages/ui/src/zag/Tooltip.jsx b/packages/ui/src/zag/Tooltip.jsx index ab5c33b23..68fe6ae8d 100644 --- a/packages/ui/src/zag/Tooltip.jsx +++ b/packages/ui/src/zag/Tooltip.jsx @@ -2,6 +2,7 @@ import * as tooltip from '@zag-js/tooltip'; import { normalizeProps, useMachine } from '@zag-js/solid'; import { createMemo, createUniqueId, Show } from 'solid-js'; import { Portal } from 'solid-js/web'; +import { Z_INDEX } from '../constants/zIndex.js'; export function Tooltip(props) { const service = useMachine(tooltip.machine, () => ({ @@ -23,7 +24,7 @@ export function Tooltip(props) { {props.children} -
      +
      diff --git a/packages/ui/src/zag/Tour.jsx b/packages/ui/src/zag/Tour.jsx index 1168ab811..e0b59e0d4 100644 --- a/packages/ui/src/zag/Tour.jsx +++ b/packages/ui/src/zag/Tour.jsx @@ -11,6 +11,7 @@ import { useContext, } from 'solid-js'; import { FiX } from 'solid-icons/fi'; +import { Z_INDEX } from '../constants/zIndex.js'; // Context for providing tour API to children const TourContext = createContext(); @@ -75,17 +76,17 @@ export function TourProvider(props) {
      -
      +
      -
      +
      diff --git a/packages/web/src/components/charts/ChartSettingsModal.jsx b/packages/web/src/components/charts/ChartSettingsModal.jsx index 15546eb73..3734ffe6e 100644 --- a/packages/web/src/components/charts/ChartSettingsModal.jsx +++ b/packages/web/src/components/charts/ChartSettingsModal.jsx @@ -1,5 +1,6 @@ import { Show, Index, createSignal } from 'solid-js'; import { BiRegularX } from 'solid-icons/bi'; +import { Z_INDEX } from '@corates/ui'; /** * ChartSettingsModal - Modal for editing chart settings like labels @@ -42,11 +43,11 @@ export default function ChartSettingsModal(props) { return ( {/* Backdrop - click to close */} -
      +
      {/* Modal */}
      diff --git a/packages/web/src/components/project-ui/ProjectView.jsx b/packages/web/src/components/project-ui/ProjectView.jsx index ac578f422..7984557e6 100644 --- a/packages/web/src/components/project-ui/ProjectView.jsx +++ b/packages/web/src/components/project-ui/ProjectView.jsx @@ -18,6 +18,7 @@ import { BiRegularHome } from 'solid-icons/bi'; import { BsListTask } from 'solid-icons/bs'; import { CgArrowsExchange } from 'solid-icons/cg'; import { AiFillCheckCircle, AiOutlineBook } from 'solid-icons/ai'; +import { isStudyInReconciliation } from '@/utils/reconciliation.js'; // Components import { ProjectProvider } from './ProjectContext.jsx'; @@ -192,23 +193,7 @@ export default function ProjectView() { }; const getReconcileCount = () => { - // Count dual-reviewer studies with at least 1 completed checklist (not completed reconciled yet) - // Includes in-progress reconciliations - return studies().filter(study => { - if (!study.reviewer1 || !study.reviewer2) return false; - const checklists = study.checklists || []; - // Skip if already has a completed reconciled checklist - if (checklists.some(c => c.isReconciled && c.status === 'completed')) return false; - const completedChecklists = checklists.filter(c => c.status === 'completed'); - // Show if 1 or 2 checklists are completed, OR if there's an in-progress reconciled checklist - const hasInProgressReconciliation = checklists.some( - c => c.isReconciled && c.status !== 'completed', - ); - return ( - hasInProgressReconciliation || - (completedChecklists.length >= 1 && completedChecklists.length <= 2) - ); - }).length; + return studies().filter(isStudyInReconciliation).length; }; // Tab configuration diff --git a/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx b/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx index bf984cda8..6763afbc4 100644 --- a/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx +++ b/packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx @@ -5,6 +5,7 @@ import ReconcileStudyCard from './ReconcileStudyCard.jsx'; import projectStore from '@/stores/projectStore.js'; import projectActionsStore from '@/stores/projectActionsStore'; import { useProjectContext } from '../ProjectContext.jsx'; +import { isStudyInReconciliation } from '@/utils/reconciliation.js'; /** * ReconcileTab - Shows studies in reconciliation workflow @@ -18,33 +19,9 @@ export default function ReconcileTab() { // Read from store directly const studies = () => projectStore.getStudies(projectId); - // Filter studies that are in reconciliation workflow: - // - Dual reviewer studies (has both reviewer1 and reviewer2) - // - Has at least 1 completed checklist - // - Does not have a completed reconciled checklist (in-progress reconciliations are shown) + // Filter studies that are in reconciliation workflow const studiesInReconciliation = createMemo(() => { - return studies().filter(study => { - // Only for dual-reviewer studies - if (!study.reviewer1 || !study.reviewer2) return false; - - const checklists = study.checklists || []; - - // Skip if already has a completed reconciled checklist - // But show if there's an in-progress reconciliation - if (checklists.some(c => c.isReconciled && c.status === 'completed')) return false; - - // Count completed checklists - const completedChecklists = checklists.filter(c => c.status === 'completed'); - - // Show if 1 or 2 checklists are completed, OR if there's an in-progress reconciled checklist - const hasInProgressReconciliation = checklists.some( - c => c.isReconciled && c.status !== 'completed', - ); - return ( - hasInProgressReconciliation || - (completedChecklists.length >= 1 && completedChecklists.length <= 2) - ); - }); + return studies().filter(isStudyInReconciliation); }); // Navigation helpers diff --git a/packages/web/src/utils/reconciliation.js b/packages/web/src/utils/reconciliation.js new file mode 100644 index 000000000..d46302b9e --- /dev/null +++ b/packages/web/src/utils/reconciliation.js @@ -0,0 +1,39 @@ +/** + * Reconciliation utility functions + * Shared logic for determining if a study is in the reconciliation workflow + */ + +/** + * Determines if a study is eligible for reconciliation + * @param {Object} study - The study object + * @returns {boolean} True if the study should be shown in reconciliation workflow + */ +export function isStudyInReconciliation(study) { + // Must have both reviewers assigned + if (!study.reviewer1 || !study.reviewer2) { + return false; + } + + const checklists = study.checklists || []; + + // Skip if already has a completed reconciled checklist + if (checklists.some(c => c.isReconciled && c.status === 'completed')) { + return false; + } + + // Count completed checklists (excluding reconciled ones) + const completedChecklists = checklists.filter(c => c.status === 'completed' && !c.isReconciled); + + // Check if there's an in-progress reconciliation + const hasInProgressReconciliation = checklists.some( + c => c.isReconciled && c.status !== 'completed', + ); + + // Show if: + // 1. There's an in-progress reconciliation, OR + // 2. There are 1 or 2 completed (non-reconciled) checklists + return ( + hasInProgressReconciliation || + (completedChecklists.length >= 1 && completedChecklists.length <= 2) + ); +}