fix(editor): Ensure page reloads after toggling prettify state (fixes #379). #380
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/StatusBar/index.tsx (1)
80-93: Optional: disable the button while toggling to prevent double clicks.If you adopt the in-flight guard in the slice, surface an
isPrettifyUpdatingflag to the UI and wire it here todisabledor a loading state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/Editor/index.tsx(2 hunks)src/components/StatusBar/index.tsx(4 hunks)src/stores/viewStore/createViewFormattingSlice.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/stores/viewStore/createViewFormattingSlice.tssrc/components/StatusBar/index.tsxsrc/components/Editor/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-04-28T08:21:28.568Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
📚 Learning: 2025-06-01T13:40:12.222Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
🧬 Code graph analysis (1)
src/stores/viewStore/createViewFormattingSlice.ts (6)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(385-385)src/typings/url.ts (1)
HASH_PARAM_NAMES(39-39)src/services/LogFileManager/index.ts (1)
setIsPrettified(199-201)src/typings/worker.ts (1)
CURSOR_CODE(101-101)src/utils/url/urlHash.ts (1)
updateViewHashParams(138-138)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
🔇 Additional comments (5)
src/stores/viewStore/createViewFormattingSlice.ts (3)
7-7: Import path verified: TheuseLogFileManagerProxyStoremodule resides atsrc/stores/logFileManagerProxyStore.ts, so the importimport useLogFileManagerProxyStore from "../logFileManagerProxyStore.ts";increateViewFormattingSlice.tsis correct.
35-35: Remove unnecessaryawaiton thesetIsPrettifiedcall increateViewFormattingSlice.ts:35—it returnsvoid, so use a direct call.⛔ Skipped due to learnings
Learnt from: Henry8192 PR: y-scope/yscope-log-viewer#367 File: src/utils/url/urlHash.ts:39-51 Timestamp: 2025-08-14T15:27:32.421Z Learning: The logFileManagerProxy methods (including setIsPrettified) are asynchronous and return Promises, requiring await when called and proper async handling.
36-39: Remove null/undefined guard—logEventNum is always a non-null number
logEventNum’s type isnumber(defaulting to 0) and cannot be null or undefined. If you need to guard against the initial “no-event” state, check for the sentinel value 0 instead.Likely an incorrect or invalid review comment.
src/components/Editor/index.tsx (1)
13-17: Centralizing toggle + error handling looks good.Importing
togglePrettifyandhandleErrorWithNotificationremoves URL/hash manipulation from the Editor and unifies behaviour.src/components/StatusBar/index.tsx (1)
47-49: Good switch to a shared toggle with centralized error handling.The
useCallbackwrapper and unified store action simplify the click path.
| const togglePrettify = async () => { | ||
| const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState(); | ||
| const newIsPrettified = !isPrettified; | ||
|
|
||
| // Update the URL and store state. | ||
| updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified}); | ||
| setIsPrettified(newIsPrettified); | ||
|
|
||
| // Update the log file manager and reload the page. | ||
| try { | ||
| const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); | ||
| await logFileManagerProxy.setIsPrettified(newIsPrettified); | ||
| await loadPageByCursor({ | ||
| code: CURSOR_CODE.EVENT_NUM, | ||
| args: {eventNum: logEventNum}, | ||
| }); | ||
| updateViewHashParams(); | ||
| } catch (error) { | ||
| handleErrorWithNotification(error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make toggle operation atomic; add re-entrancy guard and rollback on failure.
Current flow optimistically updates URL/store before I/O and doesn’t rollback on errors; it’s also re-entrant if clicked rapidly, causing racey flips and duplicate reloads.
@@
-const togglePrettify = async () => {
- const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState();
- const newIsPrettified = !isPrettified;
-
- // Update the URL and store state.
- updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified});
- setIsPrettified(newIsPrettified);
-
- // Update the log file manager and reload the page.
- try {
- const {logFileManagerProxy} = useLogFileManagerProxyStore.getState();
- await logFileManagerProxy.setIsPrettified(newIsPrettified);
- await loadPageByCursor({
- code: CURSOR_CODE.EVENT_NUM,
- args: {eventNum: logEventNum},
- });
- updateViewHashParams();
- } catch (error) {
- handleErrorWithNotification(error);
- }
-};
+let isTogglingPrettify = false;
+
+const togglePrettify = async (): Promise<void> => {
+ if (true === isTogglingPrettify) {
+ return;
+ }
+ isTogglingPrettify = true;
+ const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState();
+ const prevIsPrettified = isPrettified;
+ const newIsPrettified = false === isPrettified;
+ try {
+ // Optimistically update URL + store, then commit to services.
+ updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified});
+ setIsPrettified(newIsPrettified);
+
+ const {logFileManagerProxy} = useLogFileManagerProxyStore.getState();
+ await logFileManagerProxy.setIsPrettified(newIsPrettified);
+ if (null !== logEventNum) {
+ await loadPageByCursor({
+ code: CURSOR_CODE.EVENT_NUM,
+ args: {eventNum: logEventNum},
+ });
+ }
+ updateViewHashParams();
+ } catch (error) {
+ // Roll back URL + store to consistent previous state.
+ updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: prevIsPrettified});
+ setIsPrettified(prevIsPrettified);
+ updateViewHashParams();
+ handleErrorWithNotification(error);
+ } finally {
+ isTogglingPrettify = false;
+ }
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const togglePrettify = async () => { | |
| const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState(); | |
| const newIsPrettified = !isPrettified; | |
| // Update the URL and store state. | |
| updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified}); | |
| setIsPrettified(newIsPrettified); | |
| // Update the log file manager and reload the page. | |
| try { | |
| const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); | |
| await logFileManagerProxy.setIsPrettified(newIsPrettified); | |
| await loadPageByCursor({ | |
| code: CURSOR_CODE.EVENT_NUM, | |
| args: {eventNum: logEventNum}, | |
| }); | |
| updateViewHashParams(); | |
| } catch (error) { | |
| handleErrorWithNotification(error); | |
| } | |
| }; | |
| let isTogglingPrettify = false; | |
| const togglePrettify = async (): Promise<void> => { | |
| if (true === isTogglingPrettify) { | |
| return; | |
| } | |
| isTogglingPrettify = true; | |
| const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState(); | |
| const prevIsPrettified = isPrettified; | |
| const newIsPrettified = false === isPrettified; | |
| try { | |
| // Optimistically update URL + store, then commit to services. | |
| updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified}); | |
| setIsPrettified(newIsPrettified); | |
| const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); | |
| await logFileManagerProxy.setIsPrettified(newIsPrettified); | |
| if (null !== logEventNum) { | |
| await loadPageByCursor({ | |
| code: CURSOR_CODE.EVENT_NUM, | |
| args: {eventNum: logEventNum}, | |
| }); | |
| } | |
| updateViewHashParams(); | |
| } catch (error) { | |
| // Roll back URL + store to consistent previous state. | |
| updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: prevIsPrettified}); | |
| setIsPrettified(prevIsPrettified); | |
| updateViewHashParams(); | |
| handleErrorWithNotification(error); | |
| } finally { | |
| isTogglingPrettify = false; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/stores/viewStore/createViewFormattingSlice.ts around lines 24-44, the
togglePrettify function is optimistic and re-entrant: add a re-entrancy guard
(e.g., an in-flight flag in the view slice or a module-scoped boolean) to return
early if a toggle is already running, perform the I/O (call
logFileManagerProxy.setIsPrettified and reload) first, and only update the store
and URL after the I/O succeeds; if you must update optimistically keep the
original state and URL so you can rollback them in the catch block (call
setIsPrettified(original) and restore the previous hash param) before
re-throwing or notifying the user, and ensure the in-flight flag is cleared in
both success and error paths.
| // Update the URL and store state. | ||
| updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified}); | ||
| setIsPrettified(newIsPrettified); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid duplicative hash updates unless required.
Calling both updateWindowUrlHashParams({IS_PRETTIFIED}) and updateViewHashParams() may be redundant if the latter already covers IS_PRETTIFIED. Consider relying on a single source of truth to reduce drift.
Also applies to: 40-40
🤖 Prompt for AI Agents
In src/stores/viewStore/createViewFormattingSlice.ts around lines 28-31 (and
also line 40), there are duplicate hash updates:
updateWindowUrlHashParams({IS_PRETTIFIED}) is called in addition to
updateViewHashParams(), which risks drift; remove the redundant direct call and
rely on the single canonical updater (preferably updateViewHashParams) to write
IS_PRETTIFIED to the URL after state is set, or conversely consolidate logic so
updateWindowUrlHashParams is the single writer and updateViewHashParams no
longer writes the same key; update the surrounding code to set state first, call
only the chosen hash updater, and adjust any callers/tests expecting the
previous double-update behavior.
|
@junhaoliao I noticed that toggle prettify keyboard shortcut "Alt+Shift+F" never worked. Can you confirm this? |
On my end the shortcut is Alt + F and it seems to work for me in c404aa6 . On which platform (e.g. OS, browser) were you launching the log viewer? |
| /** | ||
| * Toggles the prettify state for formatted log viewing. | ||
| */ | ||
| const togglePrettify = () => { |
There was a problem hiding this comment.
@Henry8192 do you think there's a better place to move this into?
There was a problem hiding this comment.
since now this does not directly touch the Zustand handling of isPretified
There was a problem hiding this comment.
for now, i have moved it to the urlHash utils
There was a problem hiding this comment.
Yeah, I guess either leave it inside the urlHash utils, or we need to create a new file for this function, as it doesn't really belong to any of the utility functions.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Editor/index.tsx (1)
103-104: Off-by-one: last line is excluded from the selection.Monaco ranges are end-inclusive; using
getLineCount() - 1drops the final line.Apply:
- endLineNumber = model.getLineCount() - 1; + endLineNumber = model.getLineCount();
♻️ Duplicate comments (2)
src/components/Editor/index.tsx (1)
167-169: Keep the async IIFE here.This handler is sync; the IIFE correctly awaits and funnels errors to the notifier. Note: once togglePrettify rethrows on failure (see store review), this catch will work as intended.
src/stores/viewStore/createViewFormattingSlice.ts (1)
24-44: Make toggle atomic; add re-entrancy guard, rollback on failure, and rethrow.Current flow: (1) optimistic URL/store update, (2) service calls, (3) catch+notify but no rollback, and (4) the Promise resolves, so callers’
.catch()never runs. This can leave UI/URL inconsistent if the proxy or reload fails and can double-execute on rapid clicks.
- Add an in-flight guard to prevent concurrent toggles from Editor/StatusBar.
- Roll back URL and store on failure.
- Re-throw after rollback so callers’
.catch(handleErrorWithNotification)remains effective.- Guard
loadPageByCursorwhenlogEventNumis null.Apply:
+let isTogglingPrettify = false; + -const togglePrettify = async () => { +const togglePrettify = async (): Promise<void> => { - const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState(); - const newIsPrettified = !isPrettified; - - // Update the URL and store state. - updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified}); - setIsPrettified(newIsPrettified); - - // Update the log file manager and reload the page. - try { - const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); - await logFileManagerProxy.setIsPrettified(newIsPrettified); - await loadPageByCursor({ - code: CURSOR_CODE.EVENT_NUM, - args: {eventNum: logEventNum}, - }); - updateViewHashParams(); - } catch (error) { - handleErrorWithNotification(error); - } -}; + if (true === isTogglingPrettify) { + return; + } + isTogglingPrettify = true; + const {logEventNum, loadPageByCursor, isPrettified, setIsPrettified} = useViewStore.getState(); + const prevIsPrettified = isPrettified; + const newIsPrettified = !isPrettified; + // Optimistically update URL + store. + updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: newIsPrettified}); + setIsPrettified(newIsPrettified); + try { + const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); + logFileManagerProxy.setIsPrettified(newIsPrettified); + if (null !== logEventNum) { + await loadPageByCursor({ + code: CURSOR_CODE.EVENT_NUM, + args: {eventNum: logEventNum}, + }); + } + updateViewHashParams(); + } catch (error) { + // Roll back to previous consistent state. + updateWindowUrlHashParams({[HASH_PARAM_NAMES.IS_PRETTIFIED]: prevIsPrettified}); + setIsPrettified(prevIsPrettified); + updateViewHashParams(); + throw error; + } finally { + isTogglingPrettify = false; + } +};Optional: if
updateViewHashParams()already writesIS_PRETTIFIED, you can remove the directupdateWindowUrlHashParamscall to avoid double writes. See prior note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/Editor/index.tsx(2 hunks)src/components/StatusBar/index.tsx(4 hunks)src/stores/viewStore/createViewFormattingSlice.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/Editor/index.tsxsrc/stores/viewStore/createViewFormattingSlice.tssrc/components/StatusBar/index.tsx
🧠 Learnings (7)
📚 Learning: 2025-09-03T18:22:30.594Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#371
File: src/components/MenuBar/index.tsx:42-46
Timestamp: 2025-09-03T18:22:30.594Z
Learning: The development team has discussed that fire-and-forget async IIFE patterns (like `(async () => { ... })().catch(handleErrorWithNotification)`) don't cause synchronization issues in the yscope-log-viewer codebase due to serialized service worker communications, but they acknowledge it's still an error-prone pattern that should be refactored for better code clarity and maintainability.
Applied to files:
src/components/Editor/index.tsx
📚 Learning: 2025-09-05T15:00:08.221Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#380
File: src/components/Editor/index.tsx:167-170
Timestamp: 2025-09-05T15:00:08.221Z
Learning: In the yscope-log-viewer codebase, when calling async functions from synchronous contexts (like handleEditorCustomAction), async IIFE patterns like (async () => { await asyncFunction(); })().catch(errorHandler) are the appropriate approach to properly handle the Promise without making the parent function async.
Applied to files:
src/components/Editor/index.tsx
📚 Learning: 2025-03-03T18:10:02.451Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#196
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58
Timestamp: 2025-03-03T18:10:02.451Z
Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
📚 Learning: 2025-07-15T16:29:02.609Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#152
File: src/components/AppController.tsx:42-42
Timestamp: 2025-07-15T16:29:02.609Z
Learning: In the y-scope/yscope-log-viewer repository, the team never uses `==` for comparisons, preferring strict equality (`===`) instead. This overrides the coding guidelines that suggest using `false == <expression>` rather than `!<expression>`.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
📚 Learning: 2025-04-28T08:21:28.568Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore/loadSlice.ts:1-1
Timestamp: 2025-04-28T08:21:28.568Z
Learning: When importing from npm packages, use the public API entrypoint (e.g., `import {StateCreator} from "zustand"`) rather than deep-linking into package internals (e.g., `import {StateCreator} from "zustand/index"`), as internal file structures may change between package versions.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
📚 Learning: 2025-06-01T13:40:12.222Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
src/stores/viewStore/createViewFormattingSlice.ts
🧬 Code graph analysis (3)
src/components/Editor/index.tsx (2)
src/stores/viewStore/createViewFormattingSlice.ts (1)
togglePrettify(61-61)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
src/stores/viewStore/createViewFormattingSlice.ts (6)
src/utils/url/index.ts (1)
updateWindowUrlHashParams(389-389)src/typings/url.ts (1)
HASH_PARAM_NAMES(41-41)src/services/LogFileManager/index.ts (1)
setIsPrettified(200-202)src/typings/worker.ts (1)
CURSOR_CODE(101-101)src/utils/url/urlHash.ts (1)
updateViewHashParams(158-158)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
src/components/StatusBar/index.tsx (2)
src/stores/viewStore/createViewFormattingSlice.ts (1)
togglePrettify(61-61)src/stores/notificationStore.ts (1)
handleErrorWithNotification(76-76)
🔇 Additional comments (4)
src/components/Editor/index.tsx (1)
13-16: Imports wired correctly.Centralizing error handling and prettify toggle via store is the right direction.
src/components/StatusBar/index.tsx (2)
1-1: Imports look good and are minimally scoped.Nice cleanup.
Also applies to: 15-15, 18-18, 22-22
92-92: Hook-up is correct.onClick now routes through the shared helper with unified error handling.
src/stores/viewStore/createViewFormattingSlice.ts (1)
3-9: New deps are appropriate.Public entrypoints and typings are used correctly.
I just saw the key bindings of toggle |
i guess we can file a separate issue. it should be @coderabbitai file the issue |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you file an issue for the keyboard shortcut bug. Let me first examine the current implementation and then create the issue. @junhaoliao I've created a GitHub issue for the keyboard shortcut bug. The issue documents the problem with using The issue has been assigned to you and includes references to this PR and the specific comment thread where the bug was discovered. 🏁 Script executed: Length of output: 230 🏁 Script executed: Length of output: 3193 🏁 Script executed: Length of output: 55 @junhaoliao Perfect! I've successfully created GitHub issue #386 for the keyboard shortcut bug. Issue Details:
The issue documents the problem where The issue is now ready for you to address in a future PR, separate from the current prettify toggle functionality fix in PR #380. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/StatusBar/index.tsx (1)
73-86: Guard against rapid re-clicks; disable while in-flightDouble taps from StatusBar + Editor can race. Ensure the store’s
togglePrettifyis re-entrancy guarded and, if an in-flight flag exists, OR it intodisabledhere.Run to verify a guard exists:
#!/bin/bash rg -nC2 -g 'src/**' -P '(togglePrettify|in[Ff]light|toggling).*' src/stores/viewStore
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/StatusBar/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/components/StatusBar/index.tsx
🧬 Code graph analysis (1)
src/components/StatusBar/index.tsx (1)
src/stores/viewStore/createViewFormattingSlice.ts (1)
togglePrettify(42-42)
🔇 Additional comments (2)
src/components/StatusBar/index.tsx (2)
15-15: Centralized toggle import — LGTMCorrect import path for the shared store action; aligns with the PR objective to centralize the prettify toggle.
19-19: Permalink helper import — LGTMImport remains correct and matches the usage below.
Description
isPrettifytoggle handling into a helper function, which updates the UI visuals, URL parameters and triggers a page load.Checklist
breaking change.
Validation performed
Launched development server and tested the monaco editor action toggle as instructed by the reproduction steps in #379 :
&isPrettified=trueto the URL in the address bar and observed the logs became formatted with spaces in between numbers.F1to bring up the monaco editor's action menu. TypedView: Toggle Prettifyand hit Enter. Observed the "Pretty" button became active and the URL parameter isPrettified=true was added, and the "Prettify" toggle button became active, and the log content in the monaco editor change format as expected.isPrettified=truewas removed from the address bar, and the "Prettify" toggle button became inactive, and the log content in the monaco editor removed the special format as expected.Summary by CodeRabbit
Refactor
Bug Fixes
Documentation