feat(hooks): add console warning | task CU-86c19zx2q #3363
Conversation
…x bug with warning color in console
|
Task linked: CU-86c19zx2q [FEAT] Add console warning |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a new custom React hook Changes
Possibly related PRs
Finishing Touches
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 (
|
| @@ -105,6 +106,7 @@ const PersonsAll = () => { | |||
| <PersonsSearch /> | |||
| <Button | |||
| variant="secondary" | |||
| sx={tablet500Down && { width: '35%' }} | |||
There was a problem hiding this comment.
@FussuChalice: not sure if this is intended to be included in this PR, but anyway, this has no effect to the button currently, so it can be removed to avoid confusion later. Thanks.
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
![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 (5)
src/hooks/index.ts (1)
9-9: Maintain alphabetical order of exportsConsider moving the
useConsoleWarningexport to maintain alphabetical order with other hooks.export { default as useAppTranslation } from './useAppTranslation'; export { default as useBreakpoints } from './useBreakpoints'; +export { default as useConsoleWarning } from './useConsoleWarning'; export { default as useCurrentUser } from './useCurrentUser'; export { default as useFirebaseAuth } from './useFirebaseAuth'; export { default as useGlobal } from './useGlobal'; export { default as useInternetChecker } from './useInternetChecker'; export { default as useIntersectionObserver } from './useIntersectionObserver'; export { default as useUserAutoLogin } from './useUserAutoLogin'; -export { default as useConsoleWarning } from './useConsoleWarning';src/hooks/useConsoleWarning.tsx (4)
4-29: Add JSDoc documentation for the hookConsider adding documentation to explain the hook's purpose, usage, and when it should be used.
+/** + * A custom hook that displays a styled console warning message on component mount. + * Used to show important notices to developers in the browser console. + */ const useConsoleWarning = () => {
10-20: Move style templates outside useEffectThe style templates are static and don't need to be recreated on each effect run. Consider moving them outside the hook as constants.
+const CONSOLE_WARNING_TITLE_STYLE = ` + background: #DD8C2C; + color: #FFFFFF; + font-size: 48px; + font-weight: 700; +`; + +const CONSOLE_WARNING_DESC_STYLE = ` + font-weight: 500; + font-size: 32px; +`; const useConsoleWarning = () => { const { t } = useAppTranslation(); const warningShownRef = useRef(false); useEffect(() => { - const consoleWarningTitleStyle = ` - background: #DD8C2C; - color: #FFFFFF; - font-size: 48px; - font-weight: 700; - `; - - const consoleWarningDescStyle = ` - font-weight: 500; - font-size: 32px; - `; if (!warningShownRef.current) { - console.log(`%c${t('tr_consoleWarningTitle')}`, consoleWarningTitleStyle); - console.log(`%c${t('tr_consoleWarningDesc')}`, consoleWarningDescStyle); + console.log(`%c${t('tr_consoleWarningTitle')}`, CONSOLE_WARNING_TITLE_STYLE); + console.log(`%c${t('tr_consoleWarningDesc')}`, CONSOLE_WARNING_DESC_STYLE);
23-24: Consider using console.warn instead of console.logSince this is a warning message, using
console.warnwould be more semantically correct and provide better visibility in the console.- console.log(`%c${t('tr_consoleWarningTitle')}`, consoleWarningTitleStyle); - console.log(`%c${t('tr_consoleWarningDesc')}`, consoleWarningDescStyle); + console.warn(`%c${t('tr_consoleWarningTitle')}`, consoleWarningTitleStyle); + console.warn(`%c${t('tr_consoleWarningDesc')}`, consoleWarningDescStyle);
5-5: Type-check translation keysConsider adding type safety for translation keys to prevent runtime errors.
+type ConsoleWarningTranslationKeys = 'tr_consoleWarningTitle' | 'tr_consoleWarningDesc'; + const useConsoleWarning = () => { - const { t } = useAppTranslation(); + const { t } = useAppTranslation<ConsoleWarningTranslationKeys>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/general.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
src/hooks/index.ts(1 hunks)src/hooks/useConsoleWarning.tsx(1 hunks)src/layouts/root_layout/index.tsx(1 hunks)src/pages/persons/all_persons/index.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (2)
src/pages/persons/all_persons/index.tsx (1)
22-22: Remove unused breakpoint variableThe
tablet500Downbreakpoint is destructured but never used in the component. Consider removing it to avoid confusion.- const { desktopUp, tablet500Down } = useBreakpoints(); + const { desktopUp } = useBreakpoints();src/layouts/root_layout/index.tsx (1)
32-37: LGTM! Good placement of the console warning hookThe placement of
useConsoleWarningin the RootLayout component ensures the warning is shown once when the app starts, which is the appropriate behavior.
organized-app
|
||||||||||||||||||||||||||||
| Project |
organized-app
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 00m 05s |
| Commit |
|
| Committer | Max Makaluk |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
1
|
| View all changes introduced in this branch ↗︎ | |
Description
[FEAT] Add console warning https://app.clickup.com/t/86c19zx2q
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist: