-
Notifications
You must be signed in to change notification settings - Fork 0
[REFACTOR] 알림 기능 비활성화 #312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe code removes all logic related to notification permission handling and the associated modal from the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainPage
User->>MainPage: Visits page
Note over MainPage: Notification permission logic and modal are not triggered
MainPage-->>User: Renders page without notification-related UI
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
frontend/src/pages/MainPage/index.tsx (3)
11-11: Consider removing commented imports instead of keeping them.The commented imports for
useNotificationPermissionandHomeModalare no longer used. Consider removing these lines entirely rather than commenting them out to keep the codebase clean.-// import useNotificationPermission from '@/hooks/useNotificationPermission'; -// import HomeModal from '@/pages/PlanPage/components/HomeModal';Also applies to: 14-14
18-18: Remove commented notification-related code.The commented code blocks for modal state management and notification permission handling are no longer needed. Consider removing these commented sections to improve code readability and maintainability.
- // const [isModalOpen, setModalOpen] = useState(false); - - // const handleOpenModal = () => { - // setModalOpen(true); - // }; - - // const { requestPermission, permission, setPermission } = - // useNotificationPermission(handleOpenModal); - - // const handleCloseModal = () => { - // setModalOpen(false); - // if (permission === 'default') { - // setPermission('denied'); - // } - // }; - - // const handleConfirmModal = () => { - // requestPermission(); - // handleCloseModal(); - // };Also applies to: 20-25, 27-37
98-110: Remove commented JSX for modal rendering.The commented JSX for the
HomeModalcomponent should be removed to keep the render method clean and avoid confusion.- {/* {isModalOpen && ( - <HomeModal - title="알림 설정" - content={[ - '이동 시간이 끝나기 전에', - '알림으로 리마인드를 받아보세요! 🚀', - '놓치지 않게 도와드려요.', - ]} - close={handleCloseModal} - onConfirm={handleConfirmModal} - confirmText="알림 설정" - /> - )} */}
| import Header from '@/components/Header'; | ||
| import SearchAddress from '@/components/SearchAddress'; | ||
| import useNotificationPermission from '@/hooks/useNotificationPermission'; | ||
| // import useNotificationPermission from '@/hooks/useNotificationPermission'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify that notification-related dependencies can be removed.
Since the notification functionality has been disabled, check if the useNotificationPermission hook and HomeModal component are used elsewhere in the codebase. If not, consider removing these files and their dependencies.
Also applies to: 14-14
🏁 Script executed:
#!/bin/bash
# Description: Check if notification-related components are used elsewhere in the codebase
# Expected: No usage found, indicating these can be safely removed
echo "Checking usage of useNotificationPermission hook:"
rg -n "useNotificationPermission" --type tsx --type ts
echo -e "\nChecking usage of HomeModal component:"
rg -n "HomeModal" --type tsx --type ts
echo -e "\nChecking if these files exist and can be removed:"
fd "useNotificationPermission" --type f
fd "HomeModal" --type fLength of output: 647
🏁 Script executed:
#!/bin/bash
# Retry searching without relying on ripgrep’s built-in file types
echo "Searching for useNotificationPermission references in .ts/.tsx files:"
rg -n "useNotificationPermission" --glob "*.ts" --glob "*.tsx" || true
echo -e "\nSearching for HomeModal references in .ts/.tsx files:"
rg -n "HomeModal" --glob "*.ts" --glob "*.tsx" || trueLength of output: 2132
Remove unused notification hook and retain HomeModal
The useNotificationPermission hook is only referenced in commented lines and isn’t used anywhere else. You should:
- Delete
frontend/src/hooks/useNotificationPermission.ts - Remove the commented import and invocation in
frontend/src/pages/MainPage/index.tsx(lines 11 & 25)
The HomeModal component is still actively used throughout the app and must stay:
frontend/src/pages/PlanPage/index.tsx(import & usage at line 57)frontend/src/pages/PlanPage/components/HomeModal/index.tsxfrontend/src/pages/PlanPage/components/PlanHeader/index.tsxfrontend/src/components/Button/DeleteAccountButton/index.tsxfrontend/src/components/Button/LogoutButton/index.tsx- Additionally, if you intend to render
HomeModalinMainPage(line 99), re-enable its import at line 14.
🤖 Prompt for AI Agents
In frontend/src/pages/MainPage/index.tsx at lines 11 and 25, remove the
commented import and invocation of the unused useNotificationPermission hook.
Also, delete the file frontend/src/hooks/useNotificationPermission.ts entirely.
Ensure that the HomeModal component remains intact and imported where it is
actively used, and if you want to render HomeModal in MainPage at line 99,
re-enable its import at line 14.
Issue Number
close #311
As-Is
To-Be
Check List
Test Screenshot
(Optional) Additional Description
Summary by CodeRabbit