-
-
Notifications
You must be signed in to change notification settings - Fork 24
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: Improve Assignment Visibility with Tabs and Count Indicator #3693
feat: Improve Assignment Visibility with Tabs and Count Indicator #3693
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces several updates across components to improve responsiveness and modularity. A new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant MA as MyAssignments Component
participant UA as useMyAssignments Hook
participant T as Tabs Component
participant BP as useBreakpoints Hook
U->>MA: Interact with assignments view
MA->>UA: Request assignments data
UA->>UA: Execute filterAssignments (separates own & delegate)
UA->>UA: Execute formatAssignments (processes assignment details)
UA->>UA: Execute groupAndSortAssignments (organize assignments)
UA-->>MA: Return assignments object (own & delegate)
MA->>T: Render Tabs with assignments and actionComponent (Select dropdown)
T->>BP: Check responsive breakpoints
BP-->>T: Return current breakpoint state
T-->>MA: Display responsive tab layout with additional controls
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
✨ 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 (
|
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
🧹 Nitpick comments (4)
src/components/tabs/tab_label.tsx (1)
1-40
: Consider adding color prop for flexible badge stylingThe current implementation uses a fixed color from CSS variables. For greater flexibility, consider adding a color prop that would allow different badge colors for different contexts (e.g., warning, info, etc.).
-const TabLabel = ({ label, count }: { label: string; count: number }) => { +const TabLabel = ({ + label, + count, + badgeColor = 'var(--accent-150)' +}: { + label: string; + count: number; + badgeColor?: string; +}) => { return ( <Box sx={{ display: 'flex', alignItems: 'center', gap: '8px', transform: count === 0 && 'translateX(12px)', transition: 'transform 0.2s', }} > {label} - <LabelBadge value={count} /> + <LabelBadge value={count} backgroundColor={badgeColor} /> </Box> ); }; -const LabelBadge = ({ value }: { value: number }) => ( +const LabelBadge = ({ value, backgroundColor = 'var(--accent-150)' }: { value: number; backgroundColor?: string }) => ( <Box sx={{ - backgroundColor: 'var(--accent-150)', + backgroundColor, borderRadius: 'var(--radius-s)', width: '24px', display: 'flex', justifyContent: 'center', alignItems: 'center', height: '24px', fontSize: '14px', opacity: value === 0 ? 0 : 1, transition: 'opacity 0.2s', }} > {value.toString()} </Box> );src/features/meetings/my_assignments/index.tsx (2)
31-55
: Action component for assignment display range is straightforward.
Defining the display range in a dedicatedactionComponent
improves clarity, though consider adding some default fallback behavior if no valid option is provided.
136-140
: Responsive container layout.
The updated layout nicely ensures a column vs. row display for smaller devices.src/features/meetings/my_assignments/useAssignments.ts (1)
102-123
: Efficient grouping & sorting.
This function is clear, but if assignment volume grows, caching or more performant data structures might be necessary. For now, it’s well-structured and readable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/congregation.json
is excluded by!**/*.json
📒 Files selected for processing (7)
src/RootWrap.tsx
(1 hunks)src/components/tabs/index.tsx
(5 hunks)src/components/tabs/index.types.ts
(1 hunks)src/components/tabs/tab_label.tsx
(1 hunks)src/features/meetings/my_assignments/index.tsx
(4 hunks)src/features/meetings/my_assignments/useAssignments.ts
(2 hunks)src/features/persons/filter/index.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (22)
src/components/tabs/index.types.ts (1)
91-95
: Well-structured type definition with good documentationThe addition of the
actionComponent
property to theCustomTabProps
interface is well-documented with a clear JSDoc comment and properly typed as optional ReactNode. This maintains backward compatibility while extending the component's functionality.src/RootWrap.tsx (1)
48-58
: Explicit breakpoint keys improve theme consistency and type safetyAdding the
keys
array to the breakpoints configuration formalizes the order of breakpoints in the theme. This is a good practice that enhances type safety and ensures consistent responsive behavior across the application.src/components/tabs/index.tsx (5)
4-4
: Good import of the hook for responsive designThe addition of the
useBreakpoints
hook import enables responsive behavior for the Tabs component.
15-17
: Improved component structure with Box componentReplacing the
div
with aBox
component and adding height styling improves the layout consistency. This ensures the tab panel takes full height of its container.Also applies to: 24-24
46-48
: Updated component signature with the new propThe function signature now includes the
actionComponent
prop from the interface, and the component properly uses the new breakpoints hook to determine responsive behavior.
69-77
: Enhanced responsive layout with conditional stylingThe Box component now uses responsive styling based on the breakpoint state. The conditional styles properly handle both desktop and mobile layouts, ensuring a good user experience across device sizes.
108-108
: Simple and effective implementation of the action componentThe actionComponent is properly rendered alongside the tabs, allowing for additional UI elements to be placed in the tab header area.
src/components/tabs/tab_label.tsx (2)
3-19
: Well-designed TabLabel component with good visual feedbackThe TabLabel component effectively combines a text label with a count indicator badge. The conditional transform based on count value provides a subtle visual cue when no items are present, and the transition provides a smooth animation effect.
20-37
: Clean implementation of the badge component with conditional visibilityThe LabelBadge component is well-structured with appropriate styling for a count indicator. The conditional opacity based on the value ensures the badge is only visible when there are items to display, and the transition provides a smooth fade effect.
src/features/persons/filter/index.tsx (1)
9-9
: No issues with the new import path.
This import properly references the extractedTabLabel
component, ensuring a more modular structure.src/features/meetings/my_assignments/index.tsx (7)
2-2
: Good addition ofuseBreakpoints
.
Acquiring breakpoint info viauseBreakpoints
helps maintain responsive UI principles.
13-14
: Modular tab component usage is consistent.
The imports forTabs
andTabLabel
properly align with the new design pattern for tabbed interfaces.
26-26
: Effective destructuring ofpersonAssignments
.
SeparatingownAssignments
anddelegateAssignments
at this stage is clearer and more maintainable.
29-29
: Responsive design.
UsingtabletDown
to adapt layout shows a good approach to ensure consistent UI on various devices.
57-98
: Consider verifying scroll container design.
The75vh
height and custom scrollbar styling might cause nested scroll issues in some layouts. It may be worth reviewing if parent containers also manage scrolling or if additional responsiveness is needed (e.g., switching fromvh
to a container-based max-height).
100-111
: Tabs configuration is clear.
Having separate tabs for “My Own” and “Delegated” assignments aligns with the PR’s objective of distinct assignment visibility. The straightforward usage ofTabLabel
with counts is clear and easily maintainable.
143-143
:actionComponent
prop usage.
PassingactionComponent
into<Tabs>
fosters an extensible design, enabling additional controls alongside tab labels without extra wrapper components.src/features/meetings/my_assignments/useAssignments.ts (5)
50-59
: Filtering logic looks correct.
Your string-based date checks rely on identical date formats, which is generally safe here. However, if time zone shifts or partial-day offsets become a concern, consider normalizing dates more robustly.
60-63
:flatMap
usage efficiently combines delegate results.
This approach simplifies merges, though ensure delegate arrays are never too large to degrade performance.
64-95
:formatAssignments
refactor centralizes date handling.
Centralizing the meeting-day offset logic is a good improvement, but do confirm that each assignment key (e.g., "WM_", "MM_") consistently follows these naming conventions.
97-99
: Conditional date formatting.
Only adjusting assignment dates ifexactDate
is available avoids unnecessary computation.
125-137
: Well-structured return object.
Storing bothbyDate
andtotal
is succinct and meets the needs of your tab UI.
08c32a8
to
243f521
Compare
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
🧹 Nitpick comments (1)
src/features/meetings/my_assignments/index.tsx (1)
57-98
: Consider adding TypeScript type for assignments parameter.The renderAssignments function is well-structured and eliminates code duplication. Consider adding a TypeScript type for the assignments parameter to improve type safety.
- const renderAssignments = (assignments) => ( + const renderAssignments = (assignments: {length: number; map: Function}) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/congregation.json
is excluded by!**/*.json
📒 Files selected for processing (7)
src/RootWrap.tsx
(1 hunks)src/components/tab_label_with_badge/index.tsx
(1 hunks)src/components/tabs/index.tsx
(5 hunks)src/components/tabs/index.types.ts
(1 hunks)src/features/meetings/my_assignments/index.tsx
(4 hunks)src/features/meetings/my_assignments/useAssignments.ts
(2 hunks)src/features/persons/filter/index.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/tabs/index.types.ts
- src/RootWrap.tsx
- src/components/tabs/index.tsx
- src/features/persons/filter/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (12)
src/components/tab_label_with_badge/index.tsx (1)
22-22
: Good improvement to the badge visibility behavior.Making the badge invisible when the value is 0 while maintaining its space in the layout is an elegant way to handle the count indicator. This works well with the transition property and complements the transform behavior in the parent component.
src/features/meetings/my_assignments/index.tsx (6)
2-2
: Good addition of useBreakpoints hook for responsive design.Adding the useBreakpoints hook enhances the component's responsiveness, making it better suited for different screen sizes.
13-14
: Great use of component imports for modularity.Using the TabLabel component with badge support aligns well with the PR objectives of adding count indicators to tabs.
26-26
: Clear data restructuring for improved separation of concerns.Destructuring personAssignments into ownAssignments and delegateAssignments improves code readability and makes the distinction between the two types of assignments clearer.
31-55
: Well-structured responsive actionComponent.The actionComponent is well-implemented with responsive width and clear selection options for display range.
100-111
: Perfect implementation of tabs with count indicators.The tabs configuration is well-structured and directly fulfills the PR objective of displaying count indicators for both own and delegated assignments.
134-144
: Good responsive layout implementation.The Box styling with conditional direction based on screen size ensures a good user experience on all devices. The Tabs component with actionComponent integration is cleanly implemented.
src/features/meetings/my_assignments/useAssignments.ts (5)
50-59
: Good extraction of filtering logic.Extracting the filtering logic into a dedicated function improves code readability and maintainability.
Consider adding a return type for better type safety:
- const filterAssignments = (uid: string) => { + const filterAssignments = (uid: string): AssignmentHistoryType[] => {
61-62
: Clean implementation of assignment segregation.Separating own assignments from delegate assignments directly fulfills the PR objective of distinguishing between different types of assignments.
64-95
: Well-extracted formatting logic.Extracting the formatting logic into its own function improves code organization and follows the single responsibility principle.
Consider adding a return type for better type safety:
- const formatAssignments = (assignments: AssignmentHistoryType[]) => { + const formatAssignments = (assignments: AssignmentHistoryType[]): AssignmentHistoryType[] => {
102-123
: Clear grouping and sorting implementation.The groupAndSortAssignments function provides a clean way to organize assignments by month and sort them, making the UI presentation more structured.
Consider adding a return type for clarity:
- const groupAndSortAssignments = (assignments: AssignmentHistoryType[]) => { + const groupAndSortAssignments = (assignments: AssignmentHistoryType[]): Array<{month: string; children: AssignmentHistoryType[]}> => {
125-137
: Well-structured return objects with counts.Adding total counts to the return objects directly supports the PR objective of displaying count indicators on tabs. The structure is clean and easy to understand.
243f521
to
472d15b
Compare
472d15b
to
e45c38e
Compare
Deployment failed with the following error:
|
@ild0tt0re: the latest changes failed to deploy. Can you please update it first? Thanks. |
e45c38e
to
bff0b57
Compare
|
organized-app
|
Project |
organized-app
|
Branch Review |
main
|
Run status |
|
Run duration | 00m 05s |
Commit |
|
Committer | Angelo Annunziata |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
1
|
View all changes introduced in this branch ↗︎ |
Description
This PR introduces an improved way to separate "my own" assignments from those delegated to other publishers.
Additionally, to help prevent missing delegated assignments, a count indicator has been added to the tab. This works similarly to existing implementations (e.g., Persons → Filter, Publisher records page), showing the number of active assignments. The indicator will display either 0 or the total count of active assignments, providing an immediate overview without needing to open the tab.
Fixes # (issue)
Type of change
Checklist: