feat(native-app): inbox bulk selection #17927
Conversation
…lect to star documents
WalkthroughThe pull request implements bulk selection functionality for the inbox. It adds new localization keys in the language files ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as InboxScreen
participant AB as ActionBar
participant T as Toast
U->>S: Initiates bulk selection (select/deselect items)
S->>AB: Renders ActionBar with bulk action options
U->>AB: Presses a bulk action button (e.g., Archive)
AB->>S: Triggers corresponding bulk action callback
S->>T: Displays success or error toast notification
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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
Documentation and Community
|
|
View your CI Pipeline Execution ↗ for commit b9605ba.
☁️ Nx Cloud last updated this comment at |
| }: { | ||
| onClickStar: () => void | ||
| onClickArchive: () => void | ||
| onClickMarkAsRead: () => void |
There was a problem hiding this comment.
If we want to make this a reusable component we can remove these actions from it and extract it, I asked Daði and he did not think we would use it in another places. I wanted to try to add as little code I could to the inbox.tsx file since it is already huge so I took this approach
![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
🧹 Nitpick comments (6)
apps/native/app/src/screens/inbox/inbox.tsx (1)
847-953: Bulk actions block is functionally correct; consider refactoring to reduce repetition.
Each of the three actions (star, archive, mark as read) repeats a similar sequence: callingbulkSelectActionMutation, checkingsuccess, modifying the cache, resetting select state, and showing a toast. You could extract a helper function to streamline this logic.+ async function performBulkAction( + action: 'bookmark' | 'archive' | 'read', + onSuccessTitle: string, + onErrorTitle: string, + ) { + const result = await bulkSelectActionMutation({ + variables: { + input: { action, documentIds: selectedItems }, + }, + }) + if (result.data?.postMailActionV2?.success) { + // possibly update cache for bookmark/read + if (action !== 'archive') { + selectedItems.forEach((id) => { + client.cache.modify({ + id: client.cache.identify({ __typename: 'DocumentV2', id }), + fields: { + ...(action === 'bookmark' ? { bookmarked: () => true } : {}), + ...(action === 'read' ? { opened: () => true } : {}), + }, + }) + }) + } else { + // archive requires refetch + res.refetch() + } + resetSelectState() + showToastForBulkSelectAction({ + variant: 'success', + title: onSuccessTitle, + }) + } else { + showToastForBulkSelectAction({ + variant: 'success', + title: onErrorTitle, + message: intl.formatMessage({ + id: 'inbox.bulkSelect.pleaseTryAgain', + }), + }) + } + }apps/native/app/src/screens/inbox/components/action-bar.tsx (1)
13-25: Consider using useMemo for style objects.The style object in the Wrapper component could be memoized to prevent unnecessary recalculations.
Apply this diff to optimize performance:
+const wrapperStyle = (width: number, spacing: number) => + useMemo(() => ({ width: width - spacing }), [width, spacing]) return ( <Wrapper - style={{ width: screenWidth - theme.spacing[4] }} + style={wrapperStyle(screenWidth, theme.spacing[4])} entering={FadeInDown} exiting={FadeOutDown} >apps/native/app/src/ui/lib/list/list-item.tsx (2)
27-45: Use literal union type for icon states.The icon states could be more strictly typed to prevent invalid combinations.
Apply this diff to improve type safety:
+type IconState = + | { selectable: true; selected: boolean } + | { selectable: false; selected?: never } -const Icon = styled.View<{ - unread?: boolean - selectable?: boolean - selected?: boolean -}>` +const Icon = styled.View<IconState & { unread?: boolean }>`
72-79: Consider using a theme token for star icon dimensions.The hardcoded dimensions for the star icon should be moved to theme tokens for consistency.
Apply this diff:
+// In theme file +const iconSizes = { + small: 16, + medium: 24, + large: 50, +} - width: 16px; - height: 16px; + width: ${({ theme }) => theme.iconSizes.small}px; + height: ${({ theme }) => theme.iconSizes.small}px;apps/native/app/src/screens/inbox/inbox-filter.tsx (2)
72-80: Consider using useCallback for clearAllFilters.The clearAllFilters function should be memoized to prevent unnecessary recreations.
Apply this diff:
-const clearAllFilters = () => { +const clearAllFilters = useCallback(() => { setOpened(false) setBookmarked(false) setArchived(false) setSelectedSenders([]) setSelectedCategories([]) setDateFrom(undefined) setDateTo(undefined) -} +}, [])
82-90: Use useMemo for derived state.The isSelected calculation should be memoized to prevent unnecessary recalculations.
Apply this diff:
-const isSelected = !!( +const isSelected = useMemo(() => !!( opened || bookmarked || archived || selectedSenders.length || selectedCategories.length || dateFrom || dateTo -) +), [opened, bookmarked, archived, selectedSenders, selectedCategories, dateFrom, dateTo])
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/native/app/src/messages/en.ts(1 hunks)apps/native/app/src/messages/is.ts(1 hunks)apps/native/app/src/screens/inbox/components/action-bar.tsx(1 hunks)apps/native/app/src/screens/inbox/components/toast.tsx(1 hunks)apps/native/app/src/screens/inbox/inbox-filter.tsx(1 hunks)apps/native/app/src/screens/inbox/inbox.tsx(15 hunks)apps/native/app/src/ui/lib/card/inbox-card.tsx(2 hunks)apps/native/app/src/ui/lib/list/list-item.tsx(8 hunks)apps/native/app/src/utils/component-registry.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`apps/**/*`: "Confirm that the code adheres to the following...
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-filter.tsxapps/native/app/src/ui/lib/list/list-item.tsxapps/native/app/src/screens/inbox/components/action-bar.tsxapps/native/app/src/utils/component-registry.tsapps/native/app/src/ui/lib/card/inbox-card.tsxapps/native/app/src/screens/inbox/components/toast.tsxapps/native/app/src/screens/inbox/inbox.tsxapps/native/app/src/messages/is.tsapps/native/app/src/messages/en.ts
🔇 Additional comments (27)
apps/native/app/src/screens/inbox/inbox.tsx (16)
18-18: Import usage looks good.
No issues with addinguseNavigationButtonPress.
40-40: No immediate concerns with the new import.
The naming (usePostMailActionMutationMutation) likely comes from generated code and seems fine as is.
47-50: Registry imports are consistent.
The importedButtonRegistryandComponentRegistryappear well-organized here.
55-56: Newly imported components look correct.
BothActionBarandToastare imported appropriately for bulk selection and toast notifications.
241-250: State initialization for toast feedback is appropriate.
StoringtoastInfoandtoastVisiblein state is a clean, straightforward approach.
285-287: Bulk-selection state variables are well-defined.
DeclaringselectStateandselectedItemsensures clarity for bulk-select logic.
288-296: Conditional rightButtons for the top bar is neatly structured.
This is a good approach to toggling UI based on the selection state.
317-319: Bulk action mutation is set up properly.
This GraphQL mutation hook is a solid foundation for the upcoming bulk actions.
325-326: allDocumentsSelected logic looks safe.
Optional chaining prevents runtime errors; fallback tofalseifres.data?.documentsV2?.datais undefined.
366-380: Button press handler is well-organized.
All possible button IDs lead to distinct, readable actions. Consider guarding for unexpected button IDs if needed.
394-400: Resets selection when switching tabs.
Good user-experience move to ensure the UI remains consistent once the user leaves the inbox.
401-426: Dynamic leftButtons for Select All / Deselect All
This is a clear UX flow for toggling selection states.
477-493: Toast presentation helper is concise and effective.
It cleanly sets toast data and triggers display.
535-538: Resetting selection state is straightforward.
A single function handling both arrays and boolean resets is maintainable.
676-680: Resetting selection on search focus is a nice touch.
Prevents confusion with partially selected items if the user begins a new query.
696-697: Ensures a clean slate prior to filter navigation.
Resetting state before switching routes is consistent with the rest of the code.apps/native/app/src/ui/lib/card/inbox-card.tsx (7)
16-18: Properties added for bookmarked and selection states look correct.
These optional fields (bookmarked,selectable,selected) align with the new bulk selection feature.
20-20: Optional icon press handler is a useful addition.
Allows specialized behavior when pressing the icon while preserving compatibility for existing use cases.
26-26: DestructuringonPressIcon.
Combines well with the rest of the props for a flexible card UI.
32-32: Receivingbookmarkedprop here is consistent with the store usage.
It’s good that you coerce it to a boolean later forstarred.
34-35: Defaultingselectableandselectedto false is a safe approach.
Prevents accidental undefined behavior in the card’s logic.
49-49: Mappingbookmarkedtostarredwith a double-bang is straightforward.
Keeps theListIteminterface usage consistent.
51-53: Extension for selection logic is implemented cleanly.
Theselectable,selected, andonPressIconprops are passed down effectively toListItem.apps/native/app/src/screens/inbox/components/toast.tsx (1)
1-123: Toast component is well-structured.
- Using a variant-based config (
toastSchemes) centralizes the styling logic.- The
useEffecttimer to auto-hide is straightforward and keeps user interactions minimal.- Animations (
FadeInDown,FadeOutDown) enhance the user experience.
Overall, the component is clear, modular, and easy to integrate.apps/native/app/src/utils/component-registry.ts (1)
76-79: LGTM! Consistent button registry entries.The new bulk selection buttons follow the established naming pattern and maintain consistency with the existing registry entries.
apps/native/app/src/messages/en.ts (1)
240-254: LGTM! The bulk selection translations are well-structured and user-friendly.The translations for the bulk selection feature are clear, concise, and follow consistent patterns:
- Action buttons are clearly labeled
- Success messages confirm the actions
- Error messages provide clear feedback and next steps
apps/native/app/src/messages/is.ts (1)
240-254: LGTM! The Icelandic translations maintain consistency with the English version.The translations for the bulk selection feature are well-aligned with their English counterparts while maintaining natural Icelandic language flow:
- All 14 keys are properly translated
- The meaning and intent of each message is preserved
- The translations follow Icelandic language conventions
| <ActionBarItem | ||
| onPress={onClickStar} | ||
| underlayColor={theme.shade.shade400} | ||
| > | ||
| <ItemWrapper wrapItem={shouldWrapItems}> | ||
| <Image | ||
| source={starIcon} | ||
| style={{ | ||
| width: 16, | ||
| height: 16, | ||
| tintColor: theme.color.blue400, | ||
| }} | ||
| resizeMode="contain" | ||
| /> | ||
| <Typography variant="body3"> | ||
| <FormattedMessage | ||
| id="inbox.bulkSelectActionStar" | ||
| defaultMessage="Stjörnumerkja" | ||
| /> | ||
| </Typography> | ||
| </ItemWrapper> | ||
| </ActionBarItem> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to action buttons.
The action buttons should have proper accessibility labels to improve the user experience for screen reader users.
Apply this diff to add accessibility attributes:
<ActionBarItem
onPress={onClickStar}
underlayColor={theme.shade.shade400}
+ accessibilityRole="button"
+ accessibilityLabel={intl.formatMessage({
+ id: 'inbox.bulkSelectActionStar',
+ defaultMessage: 'Stjörnumerkja'
+ })}
>Apply similar changes to the Archive and Mark as Read buttons.
Also applies to: 94-115, 116-137
| }) | ||
| if (result.data?.postMailActionV2?.success) { | ||
| resetSelectState() | ||
| res.refetch() |
There was a problem hiding this comment.
I tried figuring out how to do this in the cache like for the other two operations but could not make it work despite following the documentation from apollo. I don't have any easy way to vizualise the cache since we upgraded to React Native 0.74 since then Flipper is not supported like it was before. Spent some time trying to set it up manually but no luck. Decided it was not worth the effort for now so refetching the data instead.
What
Add bulk selection to inbox documents.
Screen.Recording.2025-02-11.at.13.39.24.mov
Screenshots / Gifs
Checklist:
Summary by CodeRabbit