-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(native-app): app update inbox UI #15091
Conversation
…th light and dark mode
WalkthroughThe updates revolve around enhancing the user interface and logic of various components within the app. Key changes include updating constants and props in the inbox and notifications screens, refining URL handling in the organizations store, and improving the styling and structure of UI components like notification cards, headers, and list items. These modifications aim to streamline the codebase, enhance visual feedback, and ensure consistent styling across the application. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Datadog ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 3.37s Total Time |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15091 +/- ##
==========================================
- Coverage 37.13% 37.13% -0.01%
==========================================
Files 6389 6389
Lines 130008 130009 +1
Branches 37082 37082
==========================================
Hits 48274 48274
- Misses 81734 81735 +1
Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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: 0
Outside diff range and nitpick comments (1)
apps/native/app/src/ui/lib/skeleton/list-item-skeleton.tsx (1)
Line range hint
56-56
: Rename theDate
styled component to avoid shadowing the globalDate
object, which can lead to confusion and potential bugs.- const Date = styled.View` + const DateView = styled.View`
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/native/app/src/screens/inbox/inbox.tsx (5 hunks)
- apps/native/app/src/screens/notifications/notifications.tsx (1 hunks)
- apps/native/app/src/stores/organizations-store.ts (1 hunks)
- apps/native/app/src/ui/lib/card/notification-card.tsx (1 hunks)
- apps/native/app/src/ui/lib/detail/header.tsx (3 hunks)
- apps/native/app/src/ui/lib/list/list-item.tsx (3 hunks)
- apps/native/app/src/ui/lib/skeleton/list-item-skeleton.tsx (4 hunks)
Files not reviewed due to errors (5)
- apps/native/app/src/ui/lib/detail/header.tsx (no review received)
- apps/native/app/src/stores/organizations-store.ts (no review received)
- apps/native/app/src/ui/lib/list/list-item.tsx (no review received)
- apps/native/app/src/screens/notifications/notifications.tsx (no review received)
- apps/native/app/src/screens/inbox/inbox.tsx (no review received)
Files skipped from review due to trivial changes (1)
- apps/native/app/src/ui/lib/card/notification-card.tsx
Additional context used
Path-based instructions (6)
apps/native/app/src/ui/lib/skeleton/list-item-skeleton.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/ui/lib/detail/header.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/stores/organizations-store.ts (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/ui/lib/list/list-item.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/notifications/notifications.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/inbox/inbox.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Biome
apps/native/app/src/ui/lib/skeleton/list-item-skeleton.tsx
[error] 56-56: Do not shadow the global "Date" property. (lint/suspicious/noShadowRestrictedNames)
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
apps/native/app/src/stores/organizations-store.ts
[error] 33-33: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 91-91: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 97-98: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
apps/native/app/src/screens/notifications/notifications.tsx
[error] 98-114: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
[error] 129-129: This hook does not specify all of its dependencies: markAllUserNotificationsAsSeen (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
[error] 129-129: This hook does not specify all of its dependencies: updateNavigationUnseenCount (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
[error] 147-147: This hook does not specify all of its dependencies: componentId (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
[error] 147-147: This hook does not specify all of its dependencies: markUserNotificationAsRead (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
[error] 239-239: This hook specifies more dependencies than necessary: memoizedData (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
apps/native/app/src/screens/inbox/inbox.tsx
[error] 63-63: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 95-95: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 337-337: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 50-50: Shouldn't redeclare 'ListItem'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'ListItem' is defined here:
[error] 198-198: This hook specifies more dependencies than necessary: filters (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 204-204: This hook does not specify all of its dependencies: data?.totalCount (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 204-204: This hook does not specify all of its dependencies: pageSize (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
[error] 204-204: This hook does not specify all of its dependencies: getListDocument (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 204-204: This hook specifies more dependencies than necessary: refetcher (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 307-307: This hook specifies more dependencies than necessary: res.refetching (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 311-311: This hook does not specify all of its dependencies: res.refetch (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 311-311: This hook specifies more dependencies than necessary: refresh (lint/correctness/useExhaustiveDependencies)
Outer scope values aren't valid dependencies because mutating them doesn't re-render the component.
Additional comments not posted (3)
apps/native/app/src/ui/lib/skeleton/list-item-skeleton.tsx (3)
9-16
: LGTM! The dynamic theming for borders and colors in theHost
component enhances UI consistency across different themes.
25-28
: The adjustments to theCircle
component's dimensions and margins align well with the updated design specifications.
40-42
: The updated padding and margin in theContent
component improve spacing and visual consistency.
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.
LGTM 👍
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: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- apps/native/app/src/screens/notifications/notifications.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/native/app/src/screens/notifications/notifications.tsx
* feat: update UI for inbox * feat: update header in document-detail * feat: set same underlay color for both notifications and inbox for both light and dark mode * feat: add const for default page size for inbox * fix: icon for orgs in inbox on dev * feat: make icon bigger * feat: use unreadCount from server * fix: undoing changes on unreadCount variable since it changes with the filter --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: update UI for inbox * feat: update header in document-detail * feat: set same underlay color for both notifications and inbox for both light and dark mode * feat: add const for default page size for inbox * fix: icon for orgs in inbox on dev * feat: make icon bigger * feat: use unreadCount from server * fix: undoing changes on unreadCount variable since it changes with the filter --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Update UI of inbox and document-detail to match new design.
Screenshots / Gifs
Next up in a separate PR:
Checklist:
Summary by CodeRabbit
New Features
DEFAULT_PAGE_SIZE
for better pagination control.ListItem
component to support new icon types and improved styling.Bug Fixes
underlayColor
logic for consistent visual feedback inNotificationCard
.Style
Typography
for better text handling.Refactor