feat(settings): Reload file instead of the whole app when applying settings. #185
Conversation
WalkthroughThis update modifies the page refresh behavior in the settings modal workflow. The Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SD as SettingsDialog
participant SC as StateContext
participant LF as loadFile
U->>SD: Submit settings form
SD->>SC: Call loadPageByAction(RELOAD)
alt Valid refs present
SC->>LF: Invoke loadFile(fileSrc, cursor)
else
SC-->>SC: Throw error (missing fileSrcRef or logEventNumRef)
end
SD->>SD: Close settings modal
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
LGTM. An issue (or a feature request) is found during the test, which I believe we should bring that to another pr:
When the page size is set from 10000 to 1 & saved, the page size in settings is still 10000, even though the actual page size is set to 1. Should we sync up the numbers here to avoid confusion?
good catch. i was able to reproduce the issue on my end. let me see if i can quickly fix it in this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/contexts/StateContextProvider.tsx (1)
440-451: Consider adding error recovery for reload action.The current implementation throws an error if fileSrcRef or logEventNumRef is null. Consider handling this gracefully by falling back to default values or showing an error message to the user.
if (navAction.code === ACTION_NAME.RELOAD) { - if (null === fileSrcRef.current || null === logEventNumRef.current) { - throw new Error(`Unexpected fileSrc=${JSON.stringify(fileSrcRef.current) - }, logEventNum=${logEventNumRef.current} when reloading.`); - } - loadFile(fileSrcRef.current, { - code: CURSOR_CODE.EVENT_NUM, - args: {eventNum: logEventNumRef.current}, - }); + if (null === fileSrcRef.current) { + postPopUp({ + level: LOG_LEVEL.ERROR, + message: "Unable to reload: No file is currently loaded.", + timeoutMillis: DEFAULT_AUTO_DISMISS_TIMEOUT_MILLIS, + title: "Reload failed", + }); + return; + } + loadFile(fileSrcRef.current, { + code: CURSOR_CODE.EVENT_NUM, + args: {eventNum: logEventNumRef.current ?? 1}, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/modals/SettingsModal/SettingsDialog.tsx(4 hunks)src/contexts/StateContextProvider.tsx(4 hunks)src/utils/actions.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/contexts/StateContextProvider.tsxsrc/utils/actions.tssrc/components/modals/SettingsModal/SettingsDialog.tsx
🔇 Additional comments (3)
src/utils/actions.ts (1)
14-14: LGTM!The new RELOAD action follows the existing pattern and naming conventions.
Also applies to: 73-73
src/components/modals/SettingsModal/SettingsDialog.tsx (1)
21-21: LGTM!The changes improve the user experience by:
- Preserving dragged files during settings updates
- Automatically closing the modal after applying settings
- Using more accurate button text
Also applies to: 101-101, 131-132, 179-179
src/contexts/StateContextProvider.tsx (1)
271-271: LGTM!The file source caching implementation is clean and follows React best practices by using useRef.
Also applies to: 410-412
…tings # Conflicts: # src/components/modals/SettingsModal/SettingsDialog.tsx
the issue should have now been fixed. could you check again? |
Description
Checklist
breaking change.
Validation performed
logEventNum=816).10000to1, clicked "Apply" and observed the file was reloaded with only 1 log event in the view, which is Log Event 816.Summary by CodeRabbit
New Features
Bug Fixes