Skip to content
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

Merged

Conversation

ild0tt0re
Copy link
Contributor

@ild0tt0re ild0tt0re commented Feb 26, 2025

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

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Copy link

vercel bot commented Feb 26, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
staging-organized-app ✅ Ready (Inspect) Visit Preview Feb 28, 2025 7:03pm
test-organized-app ✅ Ready (Inspect) Visit Preview Feb 28, 2025 7:03pm

Copy link
Contributor

coderabbitai bot commented Feb 26, 2025

Walkthrough

This pull request introduces several updates across components to improve responsiveness and modularity. A new keys array has been added to the theme’s breakpoint configuration for an ordered definition of breakpoints. The Tabs component now accepts an additional actionComponent prop with corresponding type updates, and its inner layout adapts using a breakpoint hook. The meetings assignments feature has been refactored to clearly separate own and delegate assignments with dedicated functions for filtering, formatting, and grouping. Additionally, inline tab label components have been replaced with an external import, and badge opacity has been adjusted dynamically.

Changes

Files Change Summary
src/RootWrap.tsx Added new keys array to theme.breakpoints for an ordered definition of breakpoint keys without modifying the numeric values.
src/components/tabs/index.tsx, src/components/tabs/index.types.ts Updated the Tabs component to accept an additional actionComponent prop, integrated the useBreakpoints hook for responsive styling, and updated type definitions accordingly.
src/features/meetings/my_assignments/index.tsx, src/features/meetings/my_assignments/useAssignments.ts Refactored assignments handling by separating own and delegate assignments, introducing a Select dropdown as an action component, and modularizing filtering, formatting, and grouping logic via dedicated helper functions.
src/features/persons/filter/index.tsx, src/components/tab_label_with_badge/index.tsx Removed inline TabLabel and LabelBadge components and replaced them with an external component import; adjusted the badge’s opacity to be conditional on the count value.

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
Loading

Possibly related PRs

Suggested labels

released

Suggested reviewers

  • rhahao

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 472d15b and bff0b57.

⛔ 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 (6)
  • src/RootWrap.tsx
  • src/components/tabs/index.types.ts
  • src/components/tab_label_with_badge/index.tsx
  • src/features/meetings/my_assignments/index.tsx
  • src/features/persons/filter/index.tsx
  • src/components/tabs/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Summary
🔇 Additional comments (4)
src/features/meetings/my_assignments/useAssignments.ts (4)

50-63: Well-structured filtering with clear separation of concerns

Good refactoring of the filtering logic into a separate function. The separation of own and delegate assignments using a reusable filter function is clean and efficient.


64-95: Improve error handling for edge cases

The formatting function is well-extracted, but there's a potential issue: if neither isMidweek nor isWeekend is true, meetingDate would remain undefined when used in line 91.

Consider adding a default case or validation:

let meetingDate: Date;

+ // Default to the week's start date if neither midweek nor weekend
+ meetingDate = new Date(+year, +month - 1, +day);

if (isMidweek) {
  meetingDate = new Date(
    +year,
    +month - 1,
    +day + +midweekMeetingDay - 1
  );
}

if (isWeekend) {
  meetingDate = new Date(
    +year,
    +month - 1,
    +day + +weekendMeetingDay - 1
  );
}

125-137: Effective restructuring aligns with PR objectives

The restructured return object provides a clear separation between own and delegate assignments with their respective counts, which directly supports the PR objective of improving assignment visibility with count indicators.


102-123:

❓ Verification inconclusive

Clean implementation with potential compatibility concern

The grouping and sorting logic is well-structured. Note that toSorted() (line 119) is a newer JavaScript method that might not be supported in all browsers without polyfills or transpilation.


🏁 Script executed:

#!/bin/bash
# Check if toSorted is polyfilled or transpiled in the build process
grep -r "toSorted" package.json webpack.config.js tsconfig.json babel.config.js vite.config.ts

Length of output: 287


Action: Verify Transpilation or Polyfill for toSorted()

The grouping and sorting logic is clean, but there’s still a potential compatibility risk with using the new toSorted() method. The initial search for polyfill or transpilation configurations (e.g., in configuration files like package.json, webpack.config.js, tsconfig.json, babel.config.js, or vite.config.ts) didn’t locate any such settings. Since those files were not found, it's unclear whether the build process handles polyfilling/transpilation for this newer JavaScript feature.

Recommendations:

  • Manual Verification: Please manually check your build configuration or tooling (e.g., custom Babel setups or alternative config file names) to ensure that toSorted() is properly transpiled or polyfilled for environments that might not support it.
  • Target Environment Review: Confirm that your target browsers or runtime environments have native support for toSorted(), or add a polyfill if needed.
✨ Finishing Touches
  • 📝 Generate Docstrings

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ild0tt0re ild0tt0re changed the title feat(my_assigments): separate all assignments into "My own" and "Delegated" tabs Improve Assignment Visibility with Tabs and Count Indicator Feb 26, 2025
@ild0tt0re ild0tt0re marked this pull request as ready for review February 26, 2025 21:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 styling

The 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 dedicated actionComponent 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

📥 Commits

Reviewing files that changed from the base of the PR and between b48096f and 08c32a8.

⛔ 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 documentation

The addition of the actionComponent property to the CustomTabProps 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 safety

Adding 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 design

The addition of the useBreakpoints hook import enables responsive behavior for the Tabs component.


15-17: Improved component structure with Box component

Replacing the div with a Box 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 prop

The 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 styling

The 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 component

The 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 feedback

The 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 visibility

The 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 extracted TabLabel component, ensuring a more modular structure.

src/features/meetings/my_assignments/index.tsx (7)

2-2: Good addition of useBreakpoints.
Acquiring breakpoint info via useBreakpoints helps maintain responsive UI principles.


13-14: Modular tab component usage is consistent.
The imports for Tabs and TabLabel properly align with the new design pattern for tabbed interfaces.


26-26: Effective destructuring of personAssignments.
Separating ownAssignments and delegateAssignments at this stage is clearer and more maintainable.


29-29: Responsive design.
Using tabletDown to adapt layout shows a good approach to ensure consistent UI on various devices.


57-98: Consider verifying scroll container design.
The 75vh 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 from vh 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 of TabLabel with counts is clear and easily maintainable.


143-143: actionComponent prop usage.
Passing actionComponent 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 if exactDate is available avoids unnecessary computation.


125-137: Well-structured return object.
Storing both byDate and total is succinct and meets the needs of your tab UI.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 26, 2025
@ild0tt0re ild0tt0re changed the title Improve Assignment Visibility with Tabs and Count Indicator feat: Improve Assignment Visibility with Tabs and Count Indicator Feb 26, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 08c32a8 and 243f521.

⛔ 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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 27, 2025
Copy link

vercel bot commented Feb 27, 2025

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

@rhahao
Copy link
Member

rhahao commented Feb 28, 2025

@ild0tt0re: the latest changes failed to deploy. Can you please update it first? Thanks.

@ild0tt0re ild0tt0re force-pushed the feat/assignments-myown-and-delegated branch from e45c38e to bff0b57 Compare February 28, 2025 19:00
Copy link

@rhahao rhahao merged commit a486162 into sws2apps:main Feb 28, 2025
14 checks passed
Copy link

cypress bot commented Feb 28, 2025

organized-app    Run #2285

Run Properties:  status check passed Passed #2285  •  git commit a48616275c: feat(dashboard): group items by tab in my assignments
Project organized-app
Branch Review main
Run status status check passed Passed #2285
Run duration 00m 05s
Commit git commit a48616275c: feat(dashboard): group items by tab in my assignments
Committer Angelo Annunziata
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 1
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants