Feature/fin 021 parts/header#67
Conversation
…ponents for dev mob navigation
…r.scss added "+" button
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a new authorization provider pattern with Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Browser
participant MainLayout as MainLayout
participant AuthProvider as AuthorizedUserProvider
participant useAuthHook as useAuthorizedUser Hook
participant UserStore as useUserInformation Store
participant ProtectedPage as Protected Page Component
User->>MainLayout: Load Application
MainLayout->>AuthProvider: Wrap with Provider
activate AuthProvider
AuthProvider->>useAuthHook: Initialize Hook
activate useAuthHook
useAuthHook->>UserStore: Read userInformation
UserStore-->>useAuthHook: Return user data
alt User Exists
useAuthHook-->>AuthProvider: Return user object
AuthProvider-->>MainLayout: Provide user to tree
MainLayout->>ProtectedPage: Render with user context
ProtectedPage->>useAuthHook: Call useAuthorizedUser()
useAuthHook-->>ProtectedPage: Return authorized user
ProtectedPage-->>User: Render protected content
else User Missing
useAuthHook->>useAuthHook: Throw Error('User not found')
AuthProvider-->>User: Handle error state
end
deactivate useAuthHook
deactivate AuthProvider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/client/features/regular-incomes.tsx (1)
7-7: Placeholder content123should be replaced.This appears to be test/placeholder content that should be replaced with actual regular incomes functionality before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/features/regular-incomes.tsx` at line 7, Replace the placeholder "123" inside the div with the actual regular incomes UI: remove the test content from the div with className "flex flex-1" and render the real component or markup (e.g., map over your regular incomes data and render <RegularIncomesList /> or the appropriate component used elsewhere). Locate the JSX containing the div (className "flex flex-1") and either call the existing RegularIncomes/RegularIncomesList component or implement the mapping/rendering of the incomes array there so the placeholder is not left in production.src/client/entities/profile/mobile-navigation/navigation-bar/navigation-bar.scss (1)
1-9: Consider avoiding!importantfor padding override.Using
!importantto override button padding indicates a potential conflict with the base button styles. If feasible, consider adding a dedicated variant or modifier to the button component instead of forcing the override here.Also, the file is missing a trailing newline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/entities/profile/mobile-navigation/navigation-bar/navigation-bar.scss` around lines 1 - 9, The .nav-btn rule uses a padding: 0 !important which masks base button styles; replace this forceful override by creating/using a dedicated modifier or variant on the button (e.g., a no-padding modifier like nav-btn--no-padding or a specific utility class applied where this behavior is needed) and adjust the base button component to support that variant rather than using !important in navigation-bar.scss; also add a trailing newline to the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/main/layout.tsx`:
- Around line 6-8: The bottom-fixed UserMobileNavigationBar can overlap page
content because the main content container (the div wrapping {children}) doesn't
reserve space; update the layout in layout.tsx so the content area accounts for
the nav height—e.g., add bottom padding or a spacer equal to the bar's height to
the div that contains {children} (or apply a utility class like pb-[height] or
pb-safe-area) so content isn't obscured, and keep UserMobileNavigationBar fixed
as-is.
In `@src/app/page.tsx`:
- Around line 3-6: MainPage currently returns RegularIncomesPage without
enforcing authentication; call the existing useUserGuard() at the top of the
MainPage component (before returning JSX) to enforce redirect to
/registration/form for unauthenticated users, mirroring other pages (e.g.,
profile/page.tsx and main/page.tsx); ensure you import useUserGuard if not
already imported and keep RegularIncomesPage return unchanged so authenticated
users still render the same content.
In `@src/app/profile/layout.tsx`:
- Around line 6-8: The fixed UserMobileNavigationBar can overlap the main
content; update the children container (the div wrapping {children}) to include
a bottom inset/padding equal to the nav height (or use safe-area inset) so final
content and actions remain accessible on mobile — e.g., add a mobile-specific
bottom padding using your CSS/Tailwind utility (or env(safe-area-inset-bottom))
and ensure it only applies when UserMobileNavigationBar is present.
In
`@src/client/entities/profile/mobile-navigation/navigation-bar/navigation-bar.tsx`:
- Line 20: The active-tab check in navigation-bar uses strict equality (pathName
=== button.route), which fails for nested routes; update the matching logic in
the NavigationBar component so it marks a button active when pathName starts
with or matches the route prefix (e.g., use pathName.startsWith(button.route) or
a normalized prefix-match) instead of strict equality for computing the variant
prop for each button (the expression that currently sets variant={`${pathName
=== button.route ? 'primary' : 'muted'}`}). Ensure edge cases like root "/" are
handled by normalizing trailing slashes or requiring a "/" suffix on route
prefixes as appropriate.
- Around line 10-13: In UserMobileNavigationBar the center action button is
rendered focusable but has no onClick or accessible name; either make it a real
interactive control by adding an onClick handler and a clear accessible name
(aria-label or aria-labelledby) on the center action button JSX so
keyboard/screen-reader users can activate it, or if it is purely decorative
convert it to non-interactive (remove focusability: tabIndex={-1},
aria-hidden="true" or render as a non-button element) to remove it from the
keyboard flow; update the JSX for the "center action button" inside
UserMobileNavigationBar accordingly.
In `@src/client/features/regular-incomes.tsx`:
- Around line 5-8: The inner div has a className value with unnecessary
leading/trailing spaces (" flex flex-1 "); remove the extra whitespace so the
className is "flex flex-1" on the div that renders "123" (the element adjacent
to UserMobileNavigationBar) to ensure consistent Tailwind class parsing and
avoid accidental empty classes.
In `@src/client/shared/ui/ui-select/styles/select-item-styles.scss`:
- Around line 19-22: The CSS selector &[data-disabled="true"] doesn't match
Radix's presence-only attribute on SelectPrimitive.Item, so update the rule to
target the presence selector instead (e.g., use &[data-disabled] or
[data-disabled] on the SelectItem styles) so disabled items receive
pointer-events: none and opacity: 0.5; locate the rule in
select-item-styles.scss (the block currently using &[data-disabled="true"]) and
replace that selector with the presence-only variant.
---
Nitpick comments:
In
`@src/client/entities/profile/mobile-navigation/navigation-bar/navigation-bar.scss`:
- Around line 1-9: The .nav-btn rule uses a padding: 0 !important which masks
base button styles; replace this forceful override by creating/using a dedicated
modifier or variant on the button (e.g., a no-padding modifier like
nav-btn--no-padding or a specific utility class applied where this behavior is
needed) and adjust the base button component to support that variant rather than
using !important in navigation-bar.scss; also add a trailing newline to the
file.
In `@src/client/features/regular-incomes.tsx`:
- Line 7: Replace the placeholder "123" inside the div with the actual regular
incomes UI: remove the test content from the div with className "flex flex-1"
and render the real component or markup (e.g., map over your regular incomes
data and render <RegularIncomesList /> or the appropriate component used
elsewhere). Locate the JSX containing the div (className "flex flex-1") and
either call the existing RegularIncomes/RegularIncomesList component or
implement the mapping/rendering of the incomes array there so the placeholder is
not left in production.
🪄 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
Run ID: 4d49b8cb-c9e3-46e4-a70d-a33a443b17d1
📒 Files selected for processing (23)
src/app/main/layout.tsxsrc/app/main/page.tsxsrc/app/page.tsxsrc/app/profile/layout.tsxsrc/app/profile/page.tsxsrc/client/entities/profile/__tests__/use-user-information.store.spec.tssrc/client/entities/profile/auth-guard.hook.tssrc/client/entities/profile/mobile-navigation/navigation-bar/buttons-array-to-map.tssrc/client/entities/profile/mobile-navigation/navigation-bar/navigation-bar.scsssrc/client/entities/profile/mobile-navigation/navigation-bar/navigation-bar.tsxsrc/client/features/auth/welcome/registration-screen.tsxsrc/client/features/regular-incomes.tsxsrc/client/shared/models/size-variant.model.tssrc/client/shared/styles/design-keys/light.scsssrc/client/shared/styles/variants.scsssrc/client/shared/ui/ui-button/props/button.props.tssrc/client/shared/ui/ui-button/styles/button-variants.scsssrc/client/shared/ui/ui-button/ui-button.tsxsrc/client/shared/ui/ui-icon-button/props/icon-button.props.tssrc/client/shared/ui/ui-icon-button/styles/icon-button-variant.scsssrc/client/shared/ui/ui-icon-button/ui-icon-button.tsxsrc/client/shared/ui/ui-select/styles/select-item-styles.scsssrc/client/shared/ui/ui-svg-icon/styles/svg-icon-sizes.scss
…o catch errors refactored pages
…lick will be added
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/app/profile/main/layout.tsx (1)
1-6: Remove the'use client'directive — this layout is a pure passthrough component and does not use any client-only hooks or APIs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/profile/main/layout.tsx` around lines 1 - 6, Remove the top-level 'use client' directive from this file; UserMainPageLayout is a pure passthrough component and does not require client-side rendering. Keep the export default function UserMainPageLayout({ children }: ChildrenComponentProps) as-is and ensure any necessary imports (ChildrenComponentProps) remain, but delete the "'use client';" line.src/app/profile/settings/layout.tsx (1)
1-6: Remove unnecessary'use client'directive from this layout.This layout is nested under
src/app/profile/layout.tsx, which is already a client component. The child layout inherits the client boundary and doesn't need an explicit'use client'directive. Removing it clarifies the code without changing behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/profile/settings/layout.tsx` around lines 1 - 6, Remove the redundant client directive at the top of the file: delete the "'use client';" directive from the UserLayoutPage module so the layout inherits the client boundary from the parent profile/layout. Leave the exported component UserLayoutPage({ children }: ChildrenComponentProps) unchanged (it can continue to return children).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/profile/main/page.tsx`:
- Around line 10-57: The current page.tsx contains a large unrelated static
placeholder paragraph node that will render on /profile/main; remove that dump
and replace it with the intended localized UI content by swapping the <p>
placeholder for the proper component structure and localized strings (use your
app's i18n/translation helper or explicit props—e.g., replace the paragraph in
page.tsx with the ProfileMain component or JSX that consumes t('profile.*')
keys); ensure you don't leave any hardcoded lorem/text and add tests/static
checks to prevent shipping placeholder text.
In `@src/client/shared/services/user-information/authorized-user.hook.ts`:
- Line 9: Remove the debug logging of the full auth user object: delete the
console.log(user) statement from authorized-user.hook.ts (the code that accesses
the user variable in the hook), and if you need runtime diagnostics replace it
with a privacy-safe alternative (e.g., log only non-sensitive fields or gate
logging behind a debug flag) so no full user object is emitted to logs.
- Around line 11-13: In useAuthorizedLogic (authorized-user.hook.ts) remove the
console.log(user) and stop throwing an Error when user is null; instead return
null/undefined or a safe default so the hook does not throw during render and
lets higher-level guards like useUserGuard and AuthorizedUserInformation handle
unauthenticated state and redirects; ensure the hook's return type/signature
(useAuthorizedLogic) still matches callers so consumers can check for a falsy
user and behave accordingly.
In `@src/client/widgets/mobile-navigation/navigation-bar/navigation-bar.tsx`:
- Around line 16-27: buttonsArrayToMap contains entries for routes
"/profile/plans" and "/profile/analytics" which lead to 404s when the UiButton
onClick uses router.push(button.route); fix by either removing or disabling
those entries until pages exist: update the source that builds buttonsArrayToMap
to omit or mark entries with ids for plans/analytics, or change the UiButton
render logic in navigation-bar.tsx to conditionally render a disabled/hidden
button (check pathName, button.route, and button.id) or replace router.push with
a safe no-op for those specific routes; ensure references include
buttonsArrayToMap, UiButton, router.push and pathName so the change is easy to
locate.
---
Nitpick comments:
In `@src/app/profile/main/layout.tsx`:
- Around line 1-6: Remove the top-level 'use client' directive from this file;
UserMainPageLayout is a pure passthrough component and does not require
client-side rendering. Keep the export default function UserMainPageLayout({
children }: ChildrenComponentProps) as-is and ensure any necessary imports
(ChildrenComponentProps) remain, but delete the "'use client';" line.
In `@src/app/profile/settings/layout.tsx`:
- Around line 1-6: Remove the redundant client directive at the top of the file:
delete the "'use client';" directive from the UserLayoutPage module so the
layout inherits the client boundary from the parent profile/layout. Leave the
exported component UserLayoutPage({ children }: ChildrenComponentProps)
unchanged (it can continue to return children).
🪄 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
Run ID: ac8487da-c3cb-4627-96c2-60b022e69318
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
package.jsonsrc/app/client-layout.tsxsrc/app/init-application.tsxsrc/app/page.tsxsrc/app/profile/layout.tsxsrc/app/profile/main/layout.tsxsrc/app/profile/main/page.tsxsrc/app/profile/page.tsxsrc/app/profile/settings/layout.tsxsrc/app/profile/settings/page.tsxsrc/client/features/auth/welcome/welcome-screen.tsxsrc/client/features/regular-incomes.tsxsrc/client/shared/services/user-information/authorized-user.hook.tssrc/client/shared/ui/ui-field/styles/ui-field-label-styles.scsssrc/client/shared/ui/ui-field/styles/ui-field-title-styles.scsssrc/client/shared/ui/ui-label/styles/label-styles.scsssrc/client/widgets/mobile-navigation/navigation-bar/buttons-array-to-map.tssrc/client/widgets/mobile-navigation/navigation-bar/navigation-bar.scsssrc/client/widgets/mobile-navigation/navigation-bar/navigation-bar.tsx
✅ Files skipped from review due to trivial changes (4)
- src/app/init-application.tsx
- package.json
- src/client/widgets/mobile-navigation/navigation-bar/buttons-array-to-map.ts
- src/client/widgets/mobile-navigation/navigation-bar/navigation-bar.scss
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/page.tsx
- src/client/features/regular-incomes.tsx
- src/app/profile/page.tsx
Title
FIN-021/parts/header
Type
Description
Created component for mobile navigation UserMobileNavigationBar.tsx, which has buttons for navigating within the app. Buttons mark as active or muted accordingly to page the user is on. In this bar are used UiButtons, UiSvgIcons and UiIconButton
Media
Check list
Addition
Optional: describe any special details or important information about your code
Summary by CodeRabbit
Release Notes
New Features
Style
Refactor
Chores