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

Fixes #2986 - Multiple UI Updates #3165

Merged

Conversation

AceHunterr
Copy link
Contributor

@AceHunterr AceHunterr commented Jan 5, 2025

What kind of change does this PR introduce?
BugFix

Issue Number:
Fixes #2986

Did you add tests for your changes?
No

If relevant, did you update the documentation?
No

Summary

Made changes in the UI that created certain bugs in the Talawa Admin also included some general css changes for the repository:

  1. Fixed the overlapping of button in the organisations cards section

Before

1

Solution:

Introduced a new component for truncating text according to the width of the card
Overlapping.button.fixed.mp4
  1. Fixed non centered and overlapping profile images in the DataTable and also non-uniform length fields on smaller screens:

Before

3
2

Solution:

Used dynamic width allocation for the profile images and changed display property from flex to block for the fields for smaller screens to make the UI cleaner
peoplePageCorrected.mp4
  1. Fixed non uniform fields on the events page:

Before

eventsPageBefore

Solution:

Reduced the flex grow to 0.5 for better spacing amongst the components and did similar like above by changing display property from flex to block for smaller screens:

eventsAfter1
eventsAfter2

The main issue here was to solve for the disapperance of screen when clicked on Year view is due to the fact that Year View is not yet completed in the src\screens\OrganizationEvents\OrganizationEvents.tsx as far I could understand the code... correct me if I'm wrong here or missed out something... maybe we can have another issue to complete the year view.

  1. Fixed the white hover mixing up with the white background:

Before

5.mp4

Solution:

The major problem that arose here was the custom styles was that were given to multiple buttons and therefore a unified hover style is difficult to make... for instance there were buttons ( .btn-primary ) with white or light greyish background and white hover would look very uneasy... and if we were to change their background contrasting then there were certain buttons whose color would completely change on hover which would look too flashy and unesscary styled.

So I came up with a solution using the inset box shadow and darkening the inside of button which could be applied to our generic .btn-primary etc. buttons and white hover which would look decent.

hoverFinal.mp4

Also in the context for modifying our css for color blind users as well, we ought to move towards non-specific colors like green,blue, red so this hover effect can also enhance the hover experience for them as it darkens and lightens the shade in noticeable amount than our previous version of hover(Also with a subtle animation)

subtleAnimation.mp4

Other information
There are few changes like the hover one that would affect the entire CSS of the admin dashboard... If required I can share more snapshots or videos of the changes being taken place.

Have you read the contributing guide?
Yes

Summary by CodeRabbit

  • Style

    • Enhanced button hover effects with box shadow and background blend mode for primary, secondary, success, warning, and info buttons.
    • Updated table header colors and background.
    • Adjusted flex and responsive layout styles.
  • New Features

    • Added a new TruncatedText component for text truncation.
    • Introduced a button to display joined organizations in the user table.
    • Added no-results messaging for organization lists.
  • Improvements

    • Responsive image sizing in the organization people grid.
    • Enhanced dropdown width in the event calendar header.
    • Improved organization list card text display.
    • Added shimmer effect for loading state in organization lists.

Copy link
Contributor

coderabbitai bot commented Jan 5, 2025

Walkthrough

This pull request introduces various UI and styling enhancements across multiple components in the Talawa Admin application. Key changes include improved hover effects for buttons, the addition of a text truncation component, refined displays for organization and user lists, and enhancements to responsive design. The updates aim to create a more cohesive and visually appealing user interface by modifying CSS variables, implementing responsive image sizing, and improving text and layout handling in various components.

Changes

File Change Summary
src/assets/css/app.css Added hover styles for button classes with box shadow and background blend mode.
src/components/EventCalendar/EventHeader.tsx Added full-width inline styles to dropdown components.
src/components/OrgListCard/OrgListCard.tsx Integrated TruncatedText component for organization descriptions and addresses.
src/components/OrgListCard/TruncatedText.tsx New component for truncating text based on container width.
src/components/UsersTableItem/UsersTableItem.tsx Added button to display joined organizations.
src/screens/OrgList/OrgList.tsx Introduced no-results messaging and shimmer loading effect.
src/screens/OrganizationPeople/OrganizationPeople.tsx Enhanced profile image rendering with responsive sizing.
src/style/app.module.css Updated CSS variables and added media queries for improved responsiveness.
src/components/OrgListCard/useDebounce.tsx Introduced a new custom hook for debouncing callback functions.

Assessment against linked issues

Objective Addressed Explanation
Fix overlapping components in organization cards
Center profile images in div
Resolve mobile view field overlapping
Address hover color inconsistencies
Improve UI responsiveness

Possibly related PRs

Suggested Labels

ignore-sensitive-files-pr

Suggested Reviewers

  • palisadoes
  • disha1202
  • varshith257

Poem

🐰 Hop, hop, styling with glee,
CSS tweaks set our UI free!
Buttons shine, text truncates neat,
Responsive design, a rabbit's treat!
Talawa's look now smooth and bright,
A coding dance of pure delight! 🎨


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. (Beta)
  • @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.

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.

Copy link

github-actions bot commented Jan 5, 2025

Our Pull Request Approval Process

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

  1. We don't merge PRs with poor code quality.
    1. Follow coding best practices such that CodeRabbit.ai approves your PR.
  2. We don't merge PRs with failed tests.
    1. When tests fail, click on the Details link to learn more.
    2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
    3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
  3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

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: 1

🧹 Nitpick comments (8)
src/components/OrgListCard/TruncatedText.tsx (2)

25-25: **Evaluate the truncation coefficient more carefully **

The coefficient charPerPx = 0.065 + fontSize * 0.002 can be brittle across various fonts and screen sizes. Consider making the logic customizable or use a more robust text measurement approach (e.g., canvas-based measuring).

🧰 Tools
🪛 GitHub Actions: PR Workflow

[error] No TSDoc comment found in file. Documentation is required.


36-38: **Consider debouncing resize events **

Repeated truncation checks on every resize event can lead to unnecessary re-renders. You can optimize performance by debouncing or throttling this event handler.

-window.addEventListener('resize', truncateText);
+window.addEventListener('resize', debounce(truncateText, 200));
🧰 Tools
🪛 GitHub Actions: PR Workflow

[error] No TSDoc comment found in file. Documentation is required.

src/components/OrgListCard/OrgListCard.tsx (1)

3-3: **Remove commented-out import if not needed **

The commented-out useState import could be removed if there's no plan to use it soon, to keep imports clean.

-// import {useState} from 'react';
src/screens/OrgList/OrgList.tsx (1)

490-490: **Optional: Extract shimmer loading placeholder into its own component **

Consider extracting this repeated placeholder code into a single reusable loader component, which can be easily maintained and customized in the future.

src/style/app.module.css (2)

1011-1011: Clarify commented-out display declaration
Line 1011 comments out display: flex;. If this is intentional and no longer required, consider removing it. Otherwise, re-enable or document its usage for future reference.


1493-1513: Consider unified approach for responsive button styling
These changes introduce vertical stacking for .btn and its container in screens under max-width: 1020px. Double-check consistency with other breakpoints and confirm it meets desired design patterns (e.g., matching the approach in lines 1455-1460 and 1586-1591).

src/assets/css/app.css (2)

Line range hint 14070-14087: Effective hover implementation for button accessibility.

The new hover effect implementation using box-shadow and background-blend-mode provides good visual feedback while maintaining accessibility. The consistent white text color ensures readability across all states.

Consider adding a CSS comment documenting the accessibility benefits of this implementation for future maintainers:

.btn-primary:hover,
.btn-primary:active,
.btn-secondary:hover,
.btn-secondary:active,
.btn-success:hover,
.btn-success:active,
.btn-warning:hover,
.btn-warning:active,
.btn-info:hover,
.btn-info:active {
+ /* Enhanced hover effect using box-shadow and blend mode for better visibility,
+  * especially for users with color vision deficiencies */
  color: #fff !important;
  box-shadow: inset 50px 50px 40px rgba(0, 0, 0, 0.5);
  background-blend-mode: multiply;
}

14090-14103: Remove commented experimental code.

The commented code contains experimental HSL-based hover styles that are no longer needed since a better solution has been implemented.

Remove these unused style experiments to keep the codebase clean:

-/* .btn-primary{
-  --hover-bg: #6c757d !important; 
-}
-
-
-.btn-primary:hover,
-.btn-primary:active{
-  --hover-bg: hsl(var(--button-hue, 0), 100%, 60%) !important;
-}
-
-.btn-primary:hover,
-.btn-primary:active{
-  --hover-bg: hsl(var(--button-hue, 0), 100%, 0%) !important;
-} */
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0486d6d and b4b8eba.

📒 Files selected for processing (8)
  • src/assets/css/app.css (3 hunks)
  • src/components/EventCalendar/EventHeader.tsx (2 hunks)
  • src/components/OrgListCard/OrgListCard.tsx (2 hunks)
  • src/components/OrgListCard/TruncatedText.tsx (1 hunks)
  • src/components/UsersTableItem/UsersTableItem.tsx (1 hunks)
  • src/screens/OrgList/OrgList.tsx (2 hunks)
  • src/screens/OrganizationPeople/OrganizationPeople.tsx (1 hunks)
  • src/style/app.module.css (8 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/components/EventCalendar/EventHeader.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/OrgListCard/OrgListCard.tsx

[error] 105-105: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🪛 GitHub Actions: PR Workflow
src/components/OrgListCard/TruncatedText.tsx

[error] No TSDoc comment found in file. Documentation is required.

🔇 Additional comments (14)
src/components/OrgListCard/OrgListCard.tsx (4)

2-2: **Import statement is clear **

The new import statement for TruncatedText is consistent with the project's conventions. No changes needed.


100-100: **Consider re-enabling TruncatedText **

It looks like you are reverting to a <span> that displays the entire description. If truncation is required, uncommenting the TruncatedText usage would be essential. Please confirm whether this fallback is intentional.


103-103: **No changes at this line **


107-109: **Great usage of the TruncatedText component **

Using TruncatedText for the address fields ensures consistent styling and avoids layout breaks. Good job!

src/screens/OrganizationPeople/OrganizationPeople.tsx (2)

187-187: **Confirm column flex settings **

The column has flex: 1 and minWidth: 50. Ensure this doesn't truncate or distort images at smaller screen sizes.


189-232: **Responsive image logic looks good **

Your approach to dynamically sizing the image within the computed column width, capping it at 60px, aligns well with responsive design guidelines. The fallback Avatar usage is also well-structured.

src/screens/OrgList/OrgList.tsx (2)

401-401: **Expandable comment blocks **


403-403: **Graceful handling of “no organizations found” **

Displaying a dedicated message when no organizations match the search criteria significantly improves the user experience.

src/components/UsersTableItem/UsersTableItem.tsx (1)

164-164: Ensure consistent visual hierarchy for success buttons
This new “btn btn-success” button interacts well with the existing UI, but consider verifying that “success” (green) is reserved for positive, non-destructive actions to maintain a clear visual hierarchy.

src/style/app.module.css (5)

94-97: Double-check table header color contrast
By switching to white for both the background and text color variables (--table-head-bg and --table-header-color), there is a risk of reduced accessibility if these values blend. Verify sufficient contrast per WCAG guidelines.


Line range hint 1455-1460: Check layout shift for wide screens
Here, .btnsContainer .btnsBlock is changed to use display: block;. Ensure the new block display doesn’t unintentionally break the container’s layout when there are multiple buttons or elements side by side.


1586-1591: Maintain consistent margin reset
You reset margin-left: 0 for .btnsContainer .btnsBlock button at this breakpoint. Confirm whether this style aligns with the changes at lines 1493-1513 for uniform spacing.


5446-5446: Validate layout impact of reduced flex-grow
Reducing flex-grow from 1 to 0.5 may improve spacing, but ensure it does not cause layout truncation in smaller containers. Consider whether more granular media queries are needed.


5475-5501: Test phone-size responsiveness
This block modifies layout for max-width: 520px. Verify any forms or button sets displayed on phone screens remain readable and clickable without overflow. It might be beneficial to take UI snapshots on smaller devices to confirm.

src/components/OrgListCard/TruncatedText.tsx Show resolved Hide resolved
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: 1

🧹 Nitpick comments (3)
src/components/OrgListCard/TruncatedText.tsx (3)

30-35: Consider optimizing initial state.

Initialize truncatedText with the input text to avoid an unnecessary re-render on mount.

-  const [truncatedText, setTruncatedText] = useState<string>('');
+  const [truncatedText, setTruncatedText] = useState<string>(text);

49-50: Document or extract magic numbers used in character width calculation.

The values 0.065 and 0.002 used in the character width calculation are magic numbers. Consider extracting these as named constants with documentation explaining their derivation.

+const BASE_CHAR_WIDTH_RATIO = 0.065;
+const FONT_SIZE_MULTIPLIER = 0.002;
-        const charPerPx = 0.065 + fontSize * 0.002;
+        const charPerPx = BASE_CHAR_WIDTH_RATIO + fontSize * FONT_SIZE_MULTIPLIER;

65-69: Consider making the heading level configurable.

Using h6 might not be semantically correct for all use cases. Consider adding a prop to specify the heading level or allowing a different element type.

interface InterfaceTruncatedTextProps {
  text: string;
  maxWidthOverride?: number;
+  as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' | 'p' | 'span';
}

const TruncatedText: React.FC<InterfaceTruncatedTextProps> = ({
  text,
  maxWidthOverride,
+  as: Component = 'h6',
}) => {
  // ...
  return (
-    <h6 ref={textRef} className="text-secondary">
+    <Component ref={textRef} className="text-secondary">
      {truncatedText}
-    </h6>
+    </Component>
  );
};
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4b8eba and 4882a61.

📒 Files selected for processing (1)
  • src/components/OrgListCard/TruncatedText.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Test Application
  • GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
src/components/OrgListCard/TruncatedText.tsx (3)

1-13: LGTM! Well-structured interface with clear documentation.

The interface is properly typed and documented with clear TSDoc comments.


15-29: Excellent documentation!

The component documentation is comprehensive and includes a clear example. This successfully addresses the previous review comment about missing TSDoc.


1-72: Verify the component's behavior with organization cards.

The implementation looks solid and aligns with the PR objectives. Please ensure that the truncation behavior works correctly with various organization card widths and text lengths.

✅ Verification successful

TruncatedText component is properly integrated and used in OrgListCard

The verification shows that the TruncatedText component is correctly used in OrgListCard.tsx for truncating the organization's address. The implementation is solid with:

  • A commented-out usage for description (indicating intentional removal)
  • Active usage for formatting and truncating the address line

The component's behavior will work as expected since:

  • It's used within a card layout where space constraints are predictable
  • The address string combines multiple fields which could be lengthy
  • The component handles dynamic resizing through the resize event listener
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of TruncatedText in organization cards and related components
rg -l "TruncatedText" src/components/OrgListCard/

Length of output: 137


Script:

#!/bin/bash
# Check how TruncatedText is used in OrgListCard
rg -A 5 "TruncatedText" src/components/OrgListCard/OrgListCard.tsx

Length of output: 864

src/components/OrgListCard/TruncatedText.tsx Outdated Show resolved Hide resolved
@AceHunterr
Copy link
Contributor Author

@palisadoes I also want to get your opinion on some more such more styled CSS changes that we can made in the dashboard... like we can introduce smooth scrolling like Lenis Smooth scrolling for better UX and also slim and customised scrolling bar for like fuller view of content?

Reference for smooth scrolling : https://lenis.darkroom.engineering/

Copy link

codecov bot commented Jan 5, 2025

Codecov Report

Attention: Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.69%. Comparing base (0486d6d) to head (558b366).
Report is 6 commits behind head on develop-postgres.

Files with missing lines Patch % Lines
src/components/OrgListCard/useDebounce.tsx 50.00% 3 Missing and 1 partial ⚠️
src/components/OrgListCard/TruncatedText.tsx 91.30% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##           develop-postgres    #3165       +/-   ##
=====================================================
+ Coverage             19.97%   89.69%   +69.71%     
=====================================================
  Files                   302      325       +23     
  Lines                  7694     8487      +793     
  Branches               1678     1906      +228     
=====================================================
+ Hits                   1537     7612     +6075     
+ Misses                 6061      648     -5413     
- Partials                 96      227      +131     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@palisadoes palisadoes changed the title Issue#2986 Fixes #2986 - Multiple UI Updates Jan 5, 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 (3)
src/components/OrgListCard/useDebounce.tsx (1)

26-33: Add optional TSDoc for returned object.
The hook returns two methods, debouncedCallback and cancel. Briefly describing their usage via TSDoc will enhance readability and maintainability.

 /**
  * A custom React hook for debouncing a callback function.
  * ...
+ * @returns An object containing:
+ * - debouncedCallback: the debounced function
+ * - cancel: method to cancel a pending timeout
  */
src/components/OrgListCard/TruncatedText.tsx (2)

53-54: Validate the character width approximation.
The logic at line 53-54 calculates an approximate ratio of characters per pixel based on font size. This may cause inaccuracies with variable-width fonts or special characters. Consider measuring text in a hidden element for more consistent truncation across different fonts.


73-76: Consider accessibility and semantic heading usage.
A <h6> tag is semantically a heading. If it’s purely stylistic or repeated as a label, consider using a <div> or <span>. Additionally, for truncated text, providing a tooltip or aria-label with the full text can help screen readers.

-<h6 ref={textRef} className="text-secondary">
+<div ref={textRef} className="text-secondary" aria-label={text}>
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4882a61 and 6e6ed88.

📒 Files selected for processing (2)
  • src/components/OrgListCard/TruncatedText.tsx (1 hunks)
  • src/components/OrgListCard/useDebounce.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Test Application
  • GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
src/components/OrgListCard/useDebounce.tsx (1)

16-16: Double-check the timeout type in TypeScript.
Although using useRef<number | undefined> is valid in a DOM environment, some configurations might interpret setTimeout as returning a NodeJS.Timeout. Ensure your TS config targets the DOM definitions or consider explicitly typing it if you run into conflicts.

src/components/OrgListCard/TruncatedText.tsx (2)

1-2: Confirm SSR or static rendering usage.
This approach relies on browser APIs (window, resize event). If server-side rendering (SSR) is in play, consider conditionally using these APIs or guarding them to prevent reference errors.


64-71: Ensure multiple event listeners are not attached.
Your useEffect properly removes the event listener and cancels the debounce on unmount. This looks correct. Just make sure re-renders won't attach multiple listeners. Since you’re memoizing the callback with [debouncedCallback], you’re safe.

@AceHunterr
Copy link
Contributor Author

@coderabbitai full review

Copy link
Contributor

coderabbitai bot commented Jan 6, 2025

✅ Actions performed

Full review triggered.

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: 2

🔭 Outside diff range comments (1)
src/assets/css/app.css (1)

Line range hint 14070-14087: Review button hover styles for WCAG contrast compliance.

The current hover effect implementation:

.btn-primary:hover,
.btn-secondary:hover,
/* ... other button types ... */ {
  color: #fff !important;
  box-shadow: inset 50px 50px 40px rgba(0, 0, 0, 0.5);
  background-blend-mode: multiply;
}

While the inset shadow provides visual feedback, consider:

  1. Testing the contrast ratios with the darkened background to ensure WCAG 2.1 compliance
  2. Adding transition property for smoother hover effects

Consider this enhancement:

.btn-primary:hover,
.btn-secondary:hover,
.btn-success:hover,
.btn-warning:hover,
.btn-info:hover {
  color: #fff !important;
+ transition: all 0.2s ease-in-out;
  box-shadow: inset 50px 50px 40px rgba(0, 0, 0, 0.5);
  background-blend-mode: multiply;
+ outline: 2px solid currentColor;
+ outline-offset: 2px;
}
🧹 Nitpick comments (5)
src/screens/OrganizationPeople/OrganizationPeople.tsx (1)

191-191: Correct the comment to match the actual max size of 60px.

The inline comment says “Max size 40px,” but the code enforces a maximum of 60px (Math.min(columnWidth * 0.6, 60)). Update the comment to prevent confusion.

-        const imageSize = Math.min(columnWidth * 0.6, 60); // Max size 40px, responsive scaling
+        const imageSize = Math.min(columnWidth * 0.6, 60); // Max size 60px, responsive scaling
src/screens/OrgList/OrgList.tsx (2)

401-403: Consider extracting the conditional logic into a helper function.

Lines 401–403 handle multiple conditions for displaying a “no organizations found” message. Combining them into a small helper function or variable could improve readability and maintainability.


490-490: Explore using a built-in skeleton loader.

The shimmer effect in line 490 is visually helpful, but consider leveraging Material UI’s Skeleton or another built-in skeleton loader for consistency and maintainability. This can simplify styling and reduce custom class usage.

src/style/app.module.css (1)

5493-5501: Remove commented code.

The commented code block should be removed as it's not providing any value and may cause confusion.

-  /* .input {
-    width: 100%; 
-  }
-
-  .createButton {
-    margin: 0 auto;
-    width: 100%; 
-  } */
src/assets/css/app.css (1)

14090-14103: Remove commented code.

The commented code block contains experimental button styles that are no longer needed. This adds unnecessary complexity and maintenance overhead.

Remove the unused code block to improve maintainability:

-/* .btn-primary{
-  --hover-bg: #6c757d !important; 
-}
-
-
-.btn-primary:hover,
-.btn-primary:active{
-  --hover-bg: hsl(var(--button-hue, 0), 100%, 60%) !important;
-}
-
-.btn-primary:hover,
-.btn-primary:active{
-  --hover-bg: hsl(var(--button-hue, 0), 100%, 0%) !important;
-} */
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0486d6d and 6e6ed88.

📒 Files selected for processing (9)
  • src/assets/css/app.css (3 hunks)
  • src/components/EventCalendar/EventHeader.tsx (2 hunks)
  • src/components/OrgListCard/OrgListCard.tsx (2 hunks)
  • src/components/OrgListCard/TruncatedText.tsx (1 hunks)
  • src/components/OrgListCard/useDebounce.tsx (1 hunks)
  • src/components/UsersTableItem/UsersTableItem.tsx (1 hunks)
  • src/screens/OrgList/OrgList.tsx (2 hunks)
  • src/screens/OrganizationPeople/OrganizationPeople.tsx (1 hunks)
  • src/style/app.module.css (8 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/components/OrgListCard/TruncatedText.tsx

[warning] 39-39: src/components/OrgListCard/TruncatedText.tsx#L39
Added line #L39 was not covered by tests

src/components/OrgListCard/useDebounce.tsx

[warning] 28-29: src/components/OrgListCard/useDebounce.tsx#L28-L29
Added lines #L28 - L29 were not covered by tests

🪛 Biome (1.9.4)
src/components/OrgListCard/OrgListCard.tsx

[error] 105-105: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (7)
src/components/OrgListCard/useDebounce.tsx (1)

28-29: Increase test coverage for the debounced callback.

Lines 28–29 are not covered by tests (per static analysis). These lines invoke callback after the specified delay. Adding a test that ensures the callback is called after the delay expires would guard against regressions.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 28-29: src/components/OrgListCard/useDebounce.tsx#L28-L29
Added lines #L28 - L29 were not covered by tests

src/components/OrgListCard/TruncatedText.tsx (1)

39-39: Add a test to cover the delayed truncation scenario.

Static analysis indicates that line 39 (the delayed callback invocation) lacks test coverage. Consider adding a test that asserts the callback is triggered only after 100ms to confirm that debouncing is functioning correctly.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 39-39: src/components/OrgListCard/TruncatedText.tsx#L39
Added line #L39 was not covered by tests

src/components/EventCalendar/EventHeader.tsx (1)

75-75: LGTM! Consistent dropdown widths improve UI alignment.

The addition of width: 100% to both dropdown toggles ensures consistent width and better alignment in the header.

Also applies to: 104-104

src/components/UsersTableItem/UsersTableItem.tsx (1)

Line range hint 164-169: LGTM! Well-implemented button with proper testing and translation support.

The new button follows best practices with:

  • Proper class naming
  • Data test ID for testing
  • Internationalization support
  • Clear click handler implementation
src/style/app.module.css (3)

93-96: Improved table header contrast with subtle blue background and white text.

The color changes enhance readability and accessibility by providing better contrast between the table header background and text.


5446-5446: Improved flex item proportions with reduced flex-grow.

The change from flex-grow: 1 to flex-grow: 0.5 helps maintain better spacing and proportions between flex items.


5475-5492: Improved mobile responsiveness for calendar header.

The media query effectively handles small screens by:

  • Switching to a column layout
  • Making elements full width
  • Improving spacing between elements

src/components/OrgListCard/OrgListCard.tsx Outdated Show resolved Hide resolved
src/components/OrgListCard/OrgListCard.tsx Outdated Show resolved Hide resolved
AceHunterr and others added 3 commits January 6, 2025 14:34
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 6, 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/components/OrgListCard/OrgListCard.tsx (1)

2-3: Remove unnecessary commented-out code.

The commented-out useState import seems unused. Removing it helps keep the codebase clean.

 import React from 'react';
 import TruncatedText from './TruncatedText';
-// import {useState} from 'react';
 import FlaskIcon from 'assets/svgs/flask.svg?react';
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e6ed88 and 558b366.

📒 Files selected for processing (1)
  • src/components/OrgListCard/OrgListCard.tsx (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Test Application
  • GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
src/components/OrgListCard/OrgListCard.tsx (2)

99-103: Good job using the TruncatedText component for the organization description.

By wrapping the description in TruncatedText, you ensure it doesn't overflow or break the layout. This is consistent with previous suggestions and improves the UI.


106-106: Consistent use of optional chaining.

This addresses previous feedback for safer property access. By using address?.city and address?.line1, you mitigate potential runtime errors.

Also applies to: 108-110

@AceHunterr
Copy link
Contributor Author

Would it be possible to get some reviews on this, @palisadoes?... Actually the changes I made are in certain files that are currently being refactored as the new issues being assigned that's why I was saying to avoid the conflicts

@palisadoes palisadoes merged commit ef5a206 into PalisadoesFoundation:develop-postgres Jan 7, 2025
18 checks passed
@AceHunterr AceHunterr deleted the Issue#2986 branch January 8, 2025 18:44
Dhiren-Mhatre pushed a commit to Dhiren-Mhatre/talawa-admin that referenced this pull request Jan 9, 2025
…dation#3165)

* UI fixes on organisation pages

* Added TSDoc for Truncated Text

* Added Debouncer

* Update src/components/OrgListCard/OrgListCard.tsx

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Added code rabbit suggestions

* Fixed test error

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
rishav-jha-mech pushed a commit that referenced this pull request Jan 9, 2025
* prevented unnecessary page reload with complementary test

* Update jest.config.js

* Fixes #2986 - Multiple UI Updates (#3165)

* UI fixes on organisation pages

* Added TSDoc for Truncated Text

* Added Debouncer

* Update src/components/OrgListCard/OrgListCard.tsx

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Added code rabbit suggestions

* Fixed test error

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* refactore src/screens/OrgList from jest to vitest (#3200)

* Improved Code Coverage in src/components/Venues/VenueModal.tsx (#3203)

* Improved Code Coverage in src/components/Venues/VenueModal.tsx

* removed the ignore statements from VenueModal.tsx

* Removed istanbul ignore lines. Code coverage remians 100% (#3207)

* refactored src/screens/FundCampaignPledge from jest to vitest (#3208)

* prettier formatting and disabled ts-specific rules for js in eslint (#3186)

* Improve Code Coverage in src/screens/UserPortal/Settings/Settings.tsx (#3189)

* Preventing Overflow of images in Advertisement and Venue Post modals (#3204)

* improve code coverage of src/screens/EventManagement (#3149)

* code coverage

* jest global coverage decreased

* global jest coverage

* rename file problem solved

* changes requested resolved

* fix: update Chat section title to 'Chats' (#3216)

* removed stale comment line

* Revert "removed stale comment line"

This reverts commit e0fa894.

* removed stale comment line

---------

Co-authored-by: Peter Harrison <[email protected]>
Co-authored-by: Mehul Aggarwal <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Syed Ali Ul Hasan <[email protected]>
Co-authored-by: harshk-89 <[email protected]>
Co-authored-by: Amaan ali <[email protected]>
Co-authored-by: Yugal Sadhwani <[email protected]>
Co-authored-by: Pranav Nathe <[email protected]>
Co-authored-by: prathmesh703 <[email protected]>
Co-authored-by: Nivedita <[email protected]>
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