chore: delete UI for features that is not implemented yet#320
Merged
vitonsky merged 10 commits intoJun 9, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThis PR removes the Notifications feature and its theme wiring, simplifies activity bar and settings UI, standardizes editor toolbar button sizing to ChangesFeature Deletion and UI Simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app/src/features/NoteEditor/EditorPanel/buttons/ImageButton.tsx (1)
20-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid accepting
onClickwhen it is always ignored.The component now accepts full
ButtonProps, but internalonClickreplaces any calleronClick, which makes part of the public props misleading.Suggested fix
export const ImageButton: FC< - ButtonProps & { + Omit<ButtonProps, 'onClick'> & { onPick: (payload: InsertingPayloadMap['image']) => void; } > = ({ onPick, ...props }) => {Also applies to: 34-35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/features/NoteEditor/EditorPanel/buttons/ImageButton.tsx` around lines 20 - 24, The ImageButton component accepts all ButtonProps but always overwrites onClick, which makes the public API misleading; update the component signature to exclude onClick from the accepted props (e.g., use Omit<ButtonProps, "onClick"> or explicitly type the allowed props) so callers cannot pass an ignored onClick, and keep passing the remaining props ({...props}) into the Button; apply the same change for the other similar button declarations around lines 34-35 to remove onClick from their prop types as well.packages/app/src/features/NoteEditor/EditorPanel/buttons/LinkButton.tsx (1)
20-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
ButtonPropsoverstates supported behavior here.
onClickcan be passed by type, but it never executes because internalonClickoverwrites it. Restricting the accepted props prevents misuse.Suggested fix
export const LinkButton: FC< - ButtonProps & { + Omit<ButtonProps, 'onClick'> & { onPick: (payload: InsertingPayloadMap['link']) => void; } > = ({ onPick, ...props }) => {Also applies to: 34-35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/features/NoteEditor/EditorPanel/buttons/LinkButton.tsx` around lines 20 - 24, The component LinkButton currently types its props as ButtonProps which permits an onClick prop that never runs because the component supplies its own internal onClick; update the prop type for the LinkButton component to exclude onClick (e.g. use Omit<ButtonProps, 'onClick'>) so callers cannot pass an ignored onClick, and adjust the component signature that currently reads LinkButton: FC<ButtonProps & { onPick: ... }> to use the narrowed type (and apply the same change to the similar occurrence around the onPick usage referenced in the comment).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/src/features/NoteEditor/EditorPanel/buttons/HeaderPicker.tsx`:
- Around line 12-17: Narrow the exposed buttonProps type so consumers cannot
pass handlers that will be overridden: change the prop declaration in
HeaderPicker from buttonProps?: ButtonProps to buttonProps?: Omit<ButtonProps,
'onClick' | 'onMouseUp'> (and apply the same Omit to the other buttonProps
occurrence in this file), ensuring the component still forwards allowed props to
the internal Button/Menu.Button while preventing misleading onClick/onMouseUp
handlers from being part of the public API.
---
Outside diff comments:
In `@packages/app/src/features/NoteEditor/EditorPanel/buttons/ImageButton.tsx`:
- Around line 20-24: The ImageButton component accepts all ButtonProps but
always overwrites onClick, which makes the public API misleading; update the
component signature to exclude onClick from the accepted props (e.g., use
Omit<ButtonProps, "onClick"> or explicitly type the allowed props) so callers
cannot pass an ignored onClick, and keep passing the remaining props
({...props}) into the Button; apply the same change for the other similar button
declarations around lines 34-35 to remove onClick from their prop types as well.
In `@packages/app/src/features/NoteEditor/EditorPanel/buttons/LinkButton.tsx`:
- Around line 20-24: The component LinkButton currently types its props as
ButtonProps which permits an onClick prop that never runs because the component
supplies its own internal onClick; update the prop type for the LinkButton
component to exclude onClick (e.g. use Omit<ButtonProps, 'onClick'>) so callers
cannot pass an ignored onClick, and adjust the component signature that
currently reads LinkButton: FC<ButtonProps & { onPick: ... }> to use the
narrowed type (and apply the same change to the similar occurrence around the
onPick usage referenced in the comment).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a69b13d-aade-45dc-9d81-a0a402ad2cf3
📒 Files selected for processing (14)
packages/app/src/components/Notifications/Notifications.theme.tspackages/app/src/components/Notifications/Notifications.tsxpackages/app/src/components/theme/base.tspackages/app/src/features/App/Settings/sections/GeneralSettings.tsxpackages/app/src/features/App/Settings/sections/VaultSettings.tsxpackages/app/src/features/MainScreen/ActivityBar.tsxpackages/app/src/features/MainScreen/index.tsxpackages/app/src/features/NoteEditor/EditorPanel/EditorPanel.tsxpackages/app/src/features/NoteEditor/EditorPanel/buttons/HeaderPicker.tsxpackages/app/src/features/NoteEditor/EditorPanel/buttons/ImageButton.tsxpackages/app/src/features/NoteEditor/EditorPanel/buttons/LinkButton.tsxpackages/app/src/features/NoteEditor/NoteMenu.tsxpackages/app/src/features/NoteEditor/index.tsxpackages/app/src/features/NotificationsPopup/index.tsx
💤 Files with no reviewable changes (6)
- packages/app/src/components/Notifications/Notifications.tsx
- packages/app/src/features/NotificationsPopup/index.tsx
- packages/app/src/features/MainScreen/index.tsx
- packages/app/src/components/Notifications/Notifications.theme.ts
- packages/app/src/components/theme/base.ts
- packages/app/src/features/NoteEditor/index.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app/src/features/App/Workspace/WorkspaceStatusBarItems.tsx (1)
19-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd effect cleanup for status-bar control registration.
Line 19 registers
changeVaultbut never unregisters it. This can leave stale controls after unmount/remount cycles.Suggested fix
useEffect(() => { statusBarButtons.controls.register( 'changeVault', { visible: true, title: t('statusBar.changeVault'), onClick: () => { dispatch(workspacesApi.setActiveVault(null)); vaultControls.close(); }, icon: <FaTent />, }, { placement: 'start', priority: 1, }, ); + + return () => { + statusBarButtons.controls.unregister('changeVault'); + }; }, [dispatch, statusBarButtons.controls, t, vaultControls]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/features/App/Workspace/WorkspaceStatusBarItems.tsx` around lines 19 - 36, The effect registers the 'changeVault' control via statusBarButtons.controls.register but never unregisters it; update the useEffect to return a cleanup function that calls statusBarButtons.controls.unregister('changeVault') (or the correct deregister method on statusBarButtons.controls) so the control is removed on unmount, keeping the hook around dispatch, statusBarButtons.controls, t, and vaultControls as dependencies; place this cleanup in the same useEffect that calls statusBarButtons.controls.register.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/app/src/features/App/Workspace/WorkspaceStatusBarItems.tsx`:
- Around line 19-36: The effect registers the 'changeVault' control via
statusBarButtons.controls.register but never unregisters it; update the
useEffect to return a cleanup function that calls
statusBarButtons.controls.unregister('changeVault') (or the correct deregister
method on statusBarButtons.controls) so the control is removed on unmount,
keeping the hook around dispatch, statusBarButtons.controls, t, and
vaultControls as dependencies; place this cleanup in the same useEffect that
calls statusBarButtons.controls.register.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b27884d-0241-4a9e-9b28-8bc199e7757a
📒 Files selected for processing (4)
packages/app/src/features/App/Vault/VaultStatusBar/useVaultSyncButton.tsxpackages/app/src/features/App/Workspace/WorkspaceStatusBarItems.tsxpackages/app/src/features/App/Workspace/useActiveNoteHistoryButton.tsxpackages/app/src/hooks/useLocalizedDate.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/app/src/features/App/Vault/VaultStatusBar/useVaultSyncButton.tsx
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
1 participant
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.
Closes #143
Summary by CodeRabbit
Removed Features
New Features
UI Changes