feat: History screen implementation#21
Merged
Merged
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an exported Changes
Sequence DiagramsequenceDiagram
actor User
participant HistoryScreen
participant EditLogModal
participant Store
User->>HistoryScreen: open screen / change filter
HistoryScreen->>Store: fetch logs + profile/customCategories
Store-->>HistoryScreen: return logs/profile
HistoryScreen->>HistoryScreen: group logs by day, compute win%
HistoryScreen-->>User: render grouped, collapsible list
User->>HistoryScreen: tap entry
HistoryScreen->>EditLogModal: open with selected log
EditLogModal-->>User: show editable fields
User->>EditLogModal: update fields + Save
EditLogModal->>EditLogModal: set isSaving
EditLogModal->>HistoryScreen: onSave(id, updates)
HistoryScreen->>Store: editLog(id, updates)
Store-->>HistoryScreen: confirm save
HistoryScreen->>EditLogModal: close modal
HistoryScreen->>HistoryScreen: refresh/group logs
HistoryScreen-->>User: display updated entry
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 0/3 reviews remaining, refill in 46 minutes and 49 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/EditLogModal.tsx`:
- Around line 55-57: Update handleSave to be async with an explicit return type
(Promise<void>), set an isSaving state flag (e.g., via useState) before calling
onSave, await the call and reset isSaving after completion, and wrap the await
in try { await onSave(log.id, { note, category, context }); } catch (error:
unknown) { /* narrow error and handle/log accordingly */ } so errors are handled
and the modal only closes on success; also use isSaving to disable the Save
button and show a loading indicator.
- Around line 167-176: The save button lacks a loading state; add an isSaving
boolean state in EditLogModal and update the TouchableOpacity
(styles.saveButton, activeColor, category) so it is disabled when isSaving ||
!category, and shows a visual loading indicator (replace Text "Save changes"
with a spinner + text or alternate label) and reduced opacity while isSaving;
also update handleSave to set isSaving = true at start and reset to false in
both success and error paths to prevent duplicate submissions and provide
feedback.
- Around line 91-96: Replace the hardcoded hex colors used in the EditLogModal
render (the inline styles on styles.iconCircle and styles.readOnlyLabel where
isWin ? ... : '#DDD5C8' / '#AAA' / '#7A6654') with theme color tokens from
src/constants/theme.ts (e.g., Colors.<appropriateToken>); update the ternary
branches around isWin in the JSX that renders BrainIcon/ChipIcon and the Text
label to use those Colors tokens; if the needed tokens do not exist, add
descriptive names to theme.ts (for example sinIconBgLight, sinIconColor,
sinLabelText) and import Colors into EditLogModal.tsx before using them.
- Line 53: Replace the duplicated literal categories array in EditLogModal.tsx
by importing DEFAULT_CATEGORIES from the shared constant and using it with the
existing customCategories; specifically, remove the hard-coded
['coding','writing','planning','research','other', ...customCategories] and
instead import DEFAULT_CATEGORIES (from src/constants/theme.ts) and set
allCategories to [...DEFAULT_CATEGORIES, ...customCategories] so the component
uses the canonical list.
In `@src/screens/HistoryScreen.tsx`:
- Around line 63-66: handleSave is an async function without an explicit return
type; update its signature (the function named handleSave that calls editLog and
setEditingLog) to explicitly return Promise<void> (e.g., change to const
handleSave = async (id: string, updates: Parameters<typeof editLog>[1]):
Promise<void> => { ... }) so the async contract is explicit while keeping the
existing parameter types and behavior.
- Around line 440-447: Add new color tokens in theme.ts (e.g.,
Colors.contextWinText, Colors.contextSinBg, Colors.contextSinText) and replace
the hardcoded hex values in the HistoryScreen styles: update contextWin to use
Colors.primaryBorder and Colors.contextWinText, and update contextSin to use
Colors.contextSinBg and Colors.contextSinText; ensure you export the new tokens
from the theme file and update any imports so HistoryScreen.tsx references
Colors.<tokenName> instead of raw hex strings.
- Around line 113-114: The code uses explicit any for log items in
HistoryScreen.tsx (e.g., the entries.filter callbacks that compute wCount and
sCount); replace these any annotations with the proper LogEntry type (or omit
the annotation if entries is already LogEntry[]) so the callbacks read like
entries.filter((e: LogEntry) => e.type === 'win') or simply entries.filter(e =>
e.type === 'win'), and apply the same replacement for the other occurrences
referenced (around the lines computing sCount and the other two spots called
out), ensuring the groupedLogs-derived arrays are treated as LogEntry[] for full
type safety.
- Line 38: The calculation for yesterday using a fixed millisecond offset is
DST-unsafe; replace the line that defines the variable `yesterday` so it uses
Date#setDate() to subtract one day from a Date object (e.g., create a new
Date(), call `setDate(getDate() - 1)`, then call `toDateString()`), updating the
`yesterday` variable in HistoryScreen.tsx accordingly so day arithmetic is
DST-safe.
- Line 194: The ChipIcon in HistoryScreen is using a hardcoded color "#AAA";
replace that with the appropriate color constant exported from your theme
(import the constant from theme.ts and use it in the JSX instead of "#AAA").
Locate the JSX with <ChipIcon size={13} color="#AAA" /> and update the color
prop to use the theme export (after adding the import at the top of the file),
ensuring you reference the exact exported symbol name from
src/constants/theme.ts.
- Around line 18-254: HistoryScreen lacks an error boundary; wrap its rendered
UI in the app's ErrorBoundary (or use the withErrorBoundary HOC) so render
errors in HistoryScreen don't crash the app. Import the existing ErrorBoundary
component (or HOC) and wrap the top-level returned JSX from HistoryScreen (the
View containing the header, ScrollView, and EditLogModal) with <ErrorBoundary
fallback={...} onError={...}> (or call withErrorBoundary(HistoryScreen, {
fallback, onError }) at export) and ensure you log errors (e.g., to telemetry)
in the onError handler; keep existing props/state (filter, groupedLogs,
expandedDays, editingLog, handleSave) unchanged.
- Line 313: Replace the hardcoded "fontSize: 24" in HistoryScreen.tsx with the
FontSizes token from your theme; import { FontSizes } from src/constants/theme
(or ensure it's already imported) and use the appropriate entry (e.g.,
FontSizes.xx or FontSizes.medium) in place of 24 wherever the style or style
object contains "fontSize: 24" so all font sizing comes from the FontSizes
constant.
- Line 243: Move the static spacer inline style into the component stylesheet:
add a new entry (e.g., spacer20) to the existing StyleSheet.create() styles in
HistoryScreen (use the Spacing token, e.g., height: Spacing.small or the
appropriate Spacing value for 20), then replace the inline <View style={{
height: 20 }} /> with <View style={styles.spacer20} />; ensure only dynamic
styles remain inline and the new spacer style uses the StyleSheet-created styles
object.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c5313a0-46dc-4959-aeb1-00645606d16f
⛔ Files ignored due to path filters (1)
Build_plan.mdis excluded by!**/*.md
📒 Files selected for processing (2)
src/components/EditLogModal.tsxsrc/screens/HistoryScreen.tsx
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 12 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 12 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/EditLogModal.tsx`:
- Line 54: The merged category array allCategories in EditLogModal currently
concatenates DEFAULT_CATEGORIES and customCategories which can produce
duplicates and duplicate React keys when rendering category pills; change the
merge to produce a deduplicated list (e.g., use a Set or filter to keep unique
category identifiers/names) so that allCategories contains each category only
once and the pill-rendering logic (the map that uses the category key) gets
unique keys; ensure you dedupe by the same unique property used for rendering
keys (id or name) to avoid key collisions.
- Around line 100-101: Several UI values in EditLogModal.tsx remain hardcoded
(e.g., icon color "white" and numeric sizes like 16, 24, 48, 22, 1.5, 52) —
replace them with theme constants from src/constants/theme.ts; specifically,
change the BrainIcon and ChipIcon color argument from the literal "white" to the
appropriate Colors entry (e.g., Colors.white or Colors.iconOnPrimary), and
replace all numeric literals used for icon sizes, paddings, margins and border
widths in the styles/icon usage (refer to BrainIcon, ChipIcon,
styles.iconCircle, and any other style keys inside EditLogModal.tsx) with the
matching FontSizes, Sizes, Spacing, or BorderWidths constants; ensure you import
the required constants at the top and update all occurrences throughout the
modal (including the remaining block around the later JSX/styles) so no design
numbers are hardcoded.
In `@src/screens/HistoryScreen.tsx`:
- Around line 45-60: The grouping uses the human-facing label as the key
(variables reversedLogs, label, grouped), which drops the year and causes
distinct dates to merge; change to compute a canonical date key (e.g., ISO
yyyy-mm-dd via new Date(l.timestamp).toISOString().slice(0,10) or equivalent)
and use that key for grouped[key]. Push the log into grouped[dateKey] and
separately compute a prettyLabel (Today/Yesterday/long format) to display;
ensure the group object or a parallel map stores the prettyLabel for rendering
so expand/collapse is keyed by dateKey, not the display label.
- Around line 21-23: The component currently treats an empty logs array as "no
history" immediately, causing a flash before the query settles and masking
errors; update the rendering logic in HistoryScreen to read the loading and
error states exported by useLogs (e.g., loading or isLoading, error or isError
alongside logs and editLog) and only show the empty-state UI when loading is
false and error is falsy and logs.length === 0; likewise, when error is present,
render an error state instead of the empty copy—modify the branches that
reference logs (including the other branch that mirrors this logic) to guard on
loading/error first.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b66e8611-a5aa-4e67-bdcf-530aa5cb877a
📒 Files selected for processing (3)
src/components/EditLogModal.tsxsrc/constants/theme.tssrc/screens/HistoryScreen.tsx
| <ErrorBoundary onError={(error, errorInfo) => console.error('HistoryScreen error:', error, errorInfo)}> | ||
| <View style={styles.container}> | ||
| {/* Fixed Header */} | ||
| <View style={[styles.header, { paddingTop: Math.max(insets.top, 16) }]}> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace the remaining hardcoded design values with theme tokens.
This screen still hardcodes literals for spacing, typography, sizing, and colour (16, "white", 17, 48, 10, 30, etc.). Please route them through Spacing, FontSizes, Sizes, BorderWidths, and Colors, adding tokens in theme.ts where needed.
As per coding guidelines: "All design constants (colours, fonts, sizes, spacing) must be imported from src/constants/theme.ts; never hardcode colours, fonts, sizes, or spacing values".
Also applies to: 195-199, 279-455
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This was referenced Apr 30, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implemented the History screen for Firsthand. Includes
EditLogModal.tsxfor updating logs. Features:theme.ts(colours, fonts).PR created automatically by Jules for task 4828008898037986894 started by @FLAiRistaken
Summary by CodeRabbit