fix: (8)-refactored global style sheet #6376
Conversation
|
To generate Unit Tests for this PR, please click here. |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
WalkthroughThis pull request establishes CSS module structure for four AdminPortal screen components by creating colocated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx (1)
30-30: Pre-existing restricted import: Consider migrating in a follow-up PR.The
Modalimport fromreact-bootstrapviolates the restricted imports policy (§3.4). Per coding guidelines, useBaseModalfromshared-components/BaseModal/instead.Since this is pre-existing code and outside the scope of this CSS migration PR, consider addressing it in a separate follow-up PR.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (2)
155-158: Move enum outside component to avoid recreation on each render.The
OrganizationMembershipRoleenum is defined inside the component function, causing it to be recreated on every render. Move it outside the component or to the types file.Proposed fix
+enum OrganizationMembershipRole { + ADMIN = 'administrator', + REGULAR = 'regular', +} + function AddMember(): JSX.Element { // ... existing code ... - enum OrganizationMembershipRole { - ADMIN = 'administrator', - REGULAR = 'regular', - }
83-83: Side effect in component body.Setting
document.titledirectly in the component body is a side effect that runs on every render. Consider moving it to auseEffecthook.Proposed fix
+ useEffect(() => { + document.title = translateOrgPeople('title'); + }, [translateOrgPeople]); + - document.title = translateOrgPeople('title'); const [addUserModalisOpen, setAddUserModalIsOpen] = useState(false);
🤖 Fix all issues with AI agents
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css:
- Around line 52-62: The .membername selector has conflicting color rules: it is
set to var(--color-blue-800) in the grouped rule with .eventsAttended and then
immediately overridden to var(--color-blue-200) in the standalone .membername
rule, leaving the first declaration dead; fix by removing .membername from the
grouped selector or deleting the redundant color in the grouped rule so only the
intended color (choose var(--color-blue-200) or var(--color-blue-800)) remains
for .membername, or consolidate both rules so .eventsAttended and .membername
each have explicit, non-conflicting color declarations.
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx:
- Around line 491-574: Form fields use visual <h6> headings but lack accessible
<label> elements; add labels tied to input ids to enable screen-reader focus and
click-to-focus. For each field in AddMember.tsx (e.g., firstNameInput,
emailInput, passwordInput, confirmPasswordInput, organizationName) give the
corresponding Form.Control an id matching its data-testid and replace or wrap
the <h6> with a <label htmlFor="..."> (or add a <label> immediately before the
InputGroup) so the label is programmatically associated; keep existing onChange
handlers (handleFirstName, handleEmailChange, handlePasswordChange,
handleConfirmPasswordChange) and preserve togglePassword/toggleConfirmPassword
click behavior on InputGroup.Text. Ensure labels convey required state if needed
(aria-required or visible marker) and use visually-hidden styling only if you
must keep the current visual <h6> appearance.
- Around line 432-439: The member profile Link is building a route with only the
orgId (currentUrl) but the route requires both orgId and userId; update the Link
in AddMember.tsx (the Link element that uses currentUrl and displays
userDetails.name/emailAddress) to include userDetails.id in the pathname (e.g.
`/member/${currentUrl}/${userDetails.id}`) so each row navigates to the correct
member profile.
In @src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css:
- Around line 6-16: The .membername selector is declared twice causing color:
var(--color-blue-800) in the grouped selector ".eventsAttended, .membername" to
be immediately overridden by the later .membername color: var(--color-blue-200);
remove .membername from the grouped selector (leaving ".eventsAttended" alone)
or delete the earlier color declaration so .membername only uses
var(--color-blue-200); update the grouped rule to only target .eventsAttended
and keep the standalone .membername block as the single source of truth for that
class.
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css:
- Around line 94-100: The .orgUserTagsScrollableDiv rule uses position: sticky
but lacks a positional offset; update the CSS for .orgUserTagsScrollableDiv to
include an appropriate offset (e.g., top: 0 or top: var(--space-...)) so the
sticky behavior works as intended, or if sticking is not desired remove
position: sticky and use position: relative/initial instead; ensure the chosen
offset fits the component layout and parent's scrolling context.
- Around line 36-61: Remove the earlier duplicate `.tagsBreadCrumbs:hover` rule
block (the standalone hover at the top) and keep the consolidated
`.tagsBreadCrumbs:hover, .tagsBreadCrumbs:focus` rule that appears later; ensure
the base `.tagsBreadCrumbs` and its `&::after` pseudo-element remain unchanged
and only one hover/focus rule is present to avoid redundant definitions.
- Around line 267-290: There are duplicate CSS rules for .btnsContainer
.btnsBlock button inside the same media query causing one to override the other;
consolidate them by merging the intended margin declarations into a single
.btnsContainer .btnsBlock button rule (and remove the redundant block), ensure
any needed properties from both blocks (margin-bottom, margin-right, width,
max-width, margin-left, etc.) are preserved in that single rule, and keep the
separate .btnsContainer .btnsBlock div button rule only once as needed.
In @src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css:
- Around line 152-162: There are two conflicting .list_box rules; remove the
duplicate and consolidate into a single .list_box rule (referencing .list_box)
that keeps overflow-y: auto and uses the intended height and width — e.g., set
height: 70vh; overflow-y: auto; width: 100% (or adjust width to auto if that was
the intended behavior) so only one definition remains and no block is
overridden.
- Around line 91-94: The CSS contains duplicate rules for the selector
.btnsContainer .btnsBlock button (lines 91-94) that repeat and override the
earlier declaration in the same media query (lines ~76-80); remove the later
duplicate block and keep a single consolidated declaration for .btnsContainer
.btnsBlock button (or merge any differing properties into the original) so there
is only one source of truth for button margin styles.
- Around line 7-14: The shared CSS rule currently targets both .btnsContainer
and .calendar__controls but .calendar__controls is unused; remove the unused
selector from the rule and either keep the rule as just .btnsContainer {
display: flex; align-items: center; gap: var(--space-5); flex-shrink: 0;
white-space: nowrap; } or, if multiple components need these same properties,
extract them to a new shared class name (e.g., .flexControls) and update
component markup to use that class instead of .calendar__controls; ensure no
other references to .calendar__controls remain.
In @src/style/tokens/colors.css:
- Line 21: Replace the token --color-blue-800 value to a darker, more
desaturated blue to preserve the scale progression: update the declaration for
--color-blue-800 in src/style/tokens/colors.css (the CSS custom property name
--color-blue-800) from #0000ff to one of the recommended shades such as #003d82
or #004494 so OrganizationTags, OrganizationPeople, and AddMember use a
consistent, darker/desaturated blue.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/docs/auto-docs/screens/AdminPortal/OrganizationPeople/addMember/AddMember/functions/default.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (9)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/style/tokens/colors.css
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/style/tokens/colors.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx
🧠 Learnings (5)
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/style/tokens/colors.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx
📚 Learning: 2026-01-12T12:48:03.589Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFundCampaign/modal/CampaignModal.module.css:1-55
Timestamp: 2026-01-12T12:48:03.589Z
Learning: In CSS streamlining Phase 2, prefer co-located screen-specific CSS modules (e.g., CampaignModal.module.css next to its screen). Some duplication across screens (e.g., modal styles) is acceptable in this phase to complete screen migrations; plan to consolidate shared styles in a later phase and remove duplicates once migration completes. Reviewers should tolerate duplication for this phase but flag excessive duplication and ensure changes are well-documented.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
🧬 Code graph analysis (1)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (3)
src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(28-100)src/screens/AdminPortal/OrganizationPeople/addMember/types.ts (1)
IUserDetails(13-18)src/types/ReportingTable/utils.ts (1)
PAGE_SIZE(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (8)
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsx (1)
51-51: LGTM!The CSS module import correctly migrates from the global stylesheet to a component-scoped module, aligning with the CSS streamlining objectives.
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx (1)
37-37: LGTM!The CSS module import correctly migrates from the global stylesheet to a component-scoped module.
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx (1)
34-34: LGTM!The CSS module import correctly migrates from the global stylesheet to a component-scoped module, aligning with the CSS streamlining objectives.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css (1)
1-151: Good use of design tokens and CSS module patterns.The CSS module correctly uses design tokens for spacing, colors, typography, and shadows. The
:global(.modal-dialog)selector appropriately targets Bootstrap Modal internals. Button states and hover effects are well-defined.Based on learnings, some duplication with other screen modules (e.g.,
.removeButton,.addButton) is acceptable during CSS streamlining Phase 2.src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css (1)
45-98: Well-structured layout and avatar utilities.The responsive breakpoint for
.calendar__header, flex utilities, and avatar styling with CSS custom property fallbacks (var(--avatar-size, var(--logo-xs))) are well-implemented. Design tokens are properly used throughout.src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css (1)
152-158: Verify--border-0token value for focus state border.Line 153 uses
border: var(--border-0) solid var(--color-gray-300). If--border-0resolves to0, this border won't be visible. Verify this is the intended behavior for the focus state, or consider using--border-1instead.src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (2)
326-478: Good migration to BaseModal for Add Existing User flow.The migration from React-Bootstrap Modal to the shared BaseModal component is correctly implemented. Props like
show,onHide,title,headerTestId,className, andbackdropare properly passed. The table rendering with loading, error, and empty states follows good patterns.
479-576: Good migration to BaseModal for Create New User flow.The Create New User modal correctly uses BaseModal with
footer,showCloseButton={false},backdrop="static", andkeyboard={false}for a controlled modal experience. The footer buttons are properly extracted tocreateUserModalFooter.
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css
Outdated
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css
Outdated
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
Outdated
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
Outdated
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6376 +/- ##
========================================
Coverage 96.73% 96.73%
========================================
Files 456 456
Lines 16594 16594
Branches 5184 5184
========================================
Hits 16052 16052
Misses 529 529
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 17905 🏁 Script executed: Length of output: 3086 @Piyush-Goenka, I've completed a comprehensive review of this PR. Here are my findings: 🔴 Critical IssueIncorrect Color Token Value - The new color token Looking at your blue color scale:
The value Recommendation: Change
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (2)
129-131: Remove hardcoded user-facing string ('Member') from toast params.
NotificationToast.success(tCommon('addedSuccessfully', { item: 'Member' }))hardcodes a user-visible noun (Line 130), violating the “zero hardcoded strings” rule. Pass a translated value instead.Proposed fix (example)
NotificationToast.success( - tCommon('addedSuccessfully', { item: 'Member' }) as string, + tCommon('addedSuccessfully', { item: translateOrgPeople('member') }) as string, );
85-86: Prefer sharedPAGE_SIZEconstant to avoid divergence.Local
const PAGE_SIZE = 10;(Line 85) duplicatessrc/types/ReportingTable/utils.ts’sPAGE_SIZE. Using the shared constant reduces drift across tables.Proposed refactor
+import { PAGE_SIZE } from 'types/ReportingTable/utils'; ... - const PAGE_SIZE = 10; const [page, setPage] = useState(0);
🤖 Fix all issues with AI agents
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css:
- Around line 6-28: The .input selector is applied to a wrapper div in AddMember
(AddMember.tsx) so the rule using ".input:focus" does not trigger when the
nested input gains focus; change the selector from ".input:focus" to
".input:focus-within" in AddMember.module.css (and similarly update any other
focus-only selectors for that wrapper if present) so the intended focus styles
apply when child inputs are focused, keeping this as a CSS-only change.
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx:
- Around line 546-556: The password visibility toggle currently uses
InputGroup.Text as a clickable control (see InputGroup.Text with data-testid
"showPassword" and the togglePassword handler) but lacks keyboard and semantic
accessibility; update that element to include role="button", tabIndex={0}, an
appropriate aria-label (e.g., aria-label={showPassword ? "Hide password" : "Show
password"}), and attach a keyDown handler that calls togglePassword when Enter
or Space is pressed (in addition to the existing onClick). Apply the same
changes to the other instance (lines 572-582) to ensure both toggles are
keyboard-focusable and operable.
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css:
- Around line 87-94: The CSS rule .orgUserTagsScrollableDiv uses an invalid
single-value scrollbar-color; update the scrollbar-color declaration to include
both thumb and track colors (e.g., use two CSS variables such as
var(--color-gray-300) for the thumb and var(--color-gray-100) for the track) so
the property is valid across browsers that support it; modify the
scrollbar-color line in the .orgUserTagsScrollableDiv rule to specify both
values.
- Around line 13-21: The scrollbar-color declaration in
OrganizationTags.module.css uses a single value; update the rule that sets
scrollbar-color (wherever it appears in this module) to supply both thumb and
track colors — for example change `scrollbar-color: var(--color-gray-300);` to
`scrollbar-color: var(--color-gray-300) transparent;` (or another desired track
color) so the property uses the required two-value syntax.
In @src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css:
- Around line 1-13: Duplicate .btnsContainer rules should be consolidated into
one declaration: combine all properties from both blocks (display, margin,
align-items, gap, flex-shrink, white-space) into a single .btnsContainer rule
and remove the duplicate so behavior is unchanged; ensure var(--space-9) margin
and var(--space-5) gap are preserved and that ordering of declarations keeps any
cascade behavior intact (update any tooling/lint snapshots if needed).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧠 Learnings (6)
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
📚 Learning: 2026-01-12T18:08:53.979Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFunds/OrganizationFunds.module.css:12-18
Timestamp: 2026-01-12T18:08:53.979Z
Learning: In CSS migration PRs (e.g., the CSS Streamlining phases), limit the scope to moving styles from global stylesheets to colocated .module.css files and adopting design tokens. Do not attempt layout refactors or general CSS implementation pattern changes unless the PR explicitly states this is the goal. Preserve existing CSS behavior during migration unless a refactor is explicitly intended.
Applied to files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
📚 Learning: 2026-01-12T12:48:03.589Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFundCampaign/modal/CampaignModal.module.css:1-55
Timestamp: 2026-01-12T12:48:03.589Z
Learning: In CSS streamlining Phase 2, prefer co-located screen-specific CSS modules (e.g., CampaignModal.module.css next to its screen). Some duplication across screens (e.g., modal styles) is acceptable in this phase to complete screen migrations; plan to consolidate shared styles in a later phase and remove duplicates once migration completes. Reviewers should tolerate duplication for this phase but flag excessive duplication and ensure changes are well-documented.
Applied to files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧬 Code graph analysis (1)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (3)
src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(28-100)src/screens/AdminPortal/OrganizationPeople/addMember/types.ts (1)
IUserDetails(13-18)src/types/ReportingTable/utils.ts (1)
PAGE_SIZE(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css (1)
1-4: Verify modal width is responsive for small viewports.Hard-setting both
widthandmax-widthon.modal-dialog(Lines 2-3) can overflow on narrow screens depending on token values. Please sanity-check at common widths (e.g., 320px) to ensure no horizontal scrolling/regression. Based on learnings, preserve behavior; just verify.src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css (1)
61-74:flexColumnlikely needs pairing with adisplay: flexclass.
.flexColumn(Lines 67-69) sets onlyflex-direction. If it’s ever used standalone, it will be a no-op; ensure it’s always combined with.flexCenter(Line 61) or anotherdisplay:flexclass.src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (2)
461-479: Verify MUITablePagination count={-1}behavior for your version.
count={-1}(Line 463) is commonly used for “unknown total” server pagination, but it’s version-dependent. Please confirm it renders labels correctly and doesn’t log warnings in your MUI version.
1-600: Colocated test file exists with comprehensive coverage; no action required.The AddMember component already has a colocated test file (AddMember.spec.tsx, 44KB) with 23 test cases covering all the scenarios mentioned: modal rendering with loading/error/empty/success states (lines 1378, 1423, 1030, 867, 393), pagination logic including edge cases (lines 675, 1081, 1141, 1209), create user validation failures (lines 1030, 979, 921), and password toggle interactions (line 831).
Likely an incorrect or invalid review comment.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
Show resolved
Hide resolved
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 831 @Piyush-Goenka, I've verified the current state of the code, and unfortunately the issues I flagged are still present and have not been addressed: ❌ Issues Still Present1. Critical Issue - Color Token (Still Unfixed)File: The value is still incorrect: --color-blue-800: #0000ff;This needs to be changed to a darker shade like 2. Minor Issue - Global CSS Cleanup (Still Unfixed)File: The following sections still exist and should be removed since styles have been migrated to component modules:
📝 ClarificationYou mentioned "changes marked as addressable for the .tsx files are not meant for this PR" — but my concerns are not about .tsx files. Both issues are in CSS files:
These are directly related to this CSS refactoring PR and should be addressed before merging. Please fix these two CSS issues, and I'll be happy to approve! 🙂 |
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx (1)
103-116: Test lacks assertions—verify expected behavior.This test clicks the remove button and waits but never asserts any outcome. Without assertions, it only verifies that no exception is thrown, which provides weak coverage. Consider asserting that the mutation was called successfully or that expected UI changes occurred (e.g.,
toggleRemoveModalwas invoked, or a success indicator appeared).Suggested improvement
it('should render props and text elements test for the page component', async () => { + const toggleRemoveModal = vi.fn(() => true); const props = { - toggleRemoveModal: () => true, + toggleRemoveModal, id: '456', }; renderOrgAdminListCard(props); await wait(); await userEvent.click(screen.getByTestId(/removeAdminBtn/i)); await wait(2000); + + // Assert expected behavior after clicking remove + expect(toggleRemoveModal).toHaveBeenCalled(); });
🤖 Fix all issues with AI agents
In @src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx:
- Around line 94-101: The afterEach block currently restores window.location and
calls vi.restoreAllMocks(), but does not clear mock call history; add an
explicit vi.clearAllMocks() alongside vi.restoreAllMocks() in the afterEach to
reset mock state between tests (update the afterEach that references
originalLocation and vi.restoreAllMocks() to also call vi.clearAllMocks()).
- Around line 86-90: The test is spreading window.location (originalLocation)
into the mock which yields an empty object because Location properties are
non-enumerable; update the mock used in OrgAdminListCard.spec.tsx so instead of
using "...originalLocation" you construct an explicit Location-like object with
the required properties (e.g., pathname, href, origin, search, hash, protocol,
host) and the reload function, or create a proper Location mock via new URL(...)
and copy its string/props into the value; update the "value" assignment that
references originalLocation.pathname and reload to use this explicit mock so all
needed fields are defined.
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css:
- Around line 101-111: The CSS module rules targeting Bootstrap classes need to
use :global so they apply to externally provided class names; update the
selectors in AddMember.module.css so the Bootstrap classes are wrapped with
:global while keeping .headers local (e.g., change .headers .modal-title to
.headers :global(.modal-title), and .headers button.close / .headers .btn-close
to .headers :global(button.close) and .headers :global(.btn-close) respectively)
so the styles correctly target the Bootstrap elements.
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css:
- Around line 186-199: The CSS contains duplicate flex declarations for
.btnsContainer and .calendar__controls; consolidate by creating a single rule
that shares common properties (display: flex; align-items: center; gap;
flex-shrink; white-space) for both selectors and keep the .btnsContainer-only
property (margin: var(--space-9) 0) in a separate rule targeting only
.btnsContainer so you remove the repeated display/align declarations and
preserve the unique margin.
- Around line 49-54: Update the design token --color-blue-800 so it is darker
than --color-blue-700 (#0056b3); replace the current #0000ff value with a darker
shade (suggested #1e40af or a similar darker blue) and ensure references in the
CSS selectors .tagsBreadCrumbs:hover, .tagsBreadCrumbs:focus and the subTag link
styles use the updated token so breadcrumb hover and subTag link colors follow
the design system scale conventions.
In @src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css:
- Around line 64-68: The CSS rule for the selector ".btnsContainer .btnsBlock"
sets display: block but also includes justify-content: space-between, which is
ineffective on block-level elements; remove the justify-content declaration (or
if the intent was horizontal distribution, change display to flex on .btnsBlock)
so either drop justify-content or switch display to flex to make justify-content
apply.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
src/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
All components must follow reusable component policy: No inline interfaces, correct portal placement, wrapper usage
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
🧠 Learnings (11)
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
📚 Learning: 2026-01-12T18:08:53.979Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFunds/OrganizationFunds.module.css:12-18
Timestamp: 2026-01-12T18:08:53.979Z
Learning: In CSS migration PRs (e.g., the CSS Streamlining phases), limit the scope to moving styles from global stylesheets to colocated .module.css files and adopting design tokens. Do not attempt layout refactors or general CSS implementation pattern changes unless the PR explicitly states this is the goal. Preserve existing CSS behavior during migration unless a refactor is explicitly intended.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
📚 Learning: 2026-01-12T12:48:03.589Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFundCampaign/modal/CampaignModal.module.css:1-55
Timestamp: 2026-01-12T12:48:03.589Z
Learning: In CSS streamlining Phase 2, prefer co-located screen-specific CSS modules (e.g., CampaignModal.module.css next to its screen). Some duplication across screens (e.g., modal styles) is acceptable in this phase to complete screen migrations; plan to consolidate shared styles in a later phase and remove duplicates once migration completes. Reviewers should tolerate duplication for this phase but flag excessive duplication and ensure changes are well-documented.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-08T16:45:49.201Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6241
File: src/components/AdminPortal/OrgPeopleListCard/OrgPeopleListCard.tsx:12-16
Timestamp: 2026-01-08T16:45:49.201Z
Learning: In TSX components, prefer flat JSDoc parameter documentation format (e.g., param id, param toggleRemoveModal) over the nested format (param props.id). The nested form can break editor parsing. When a component accepts a props object, destructure the props in the function signature to align with the flat JSDoc style (e.g., function MyComp({ id, toggleRemoveModal }: Props) { ... }). Ensure all param tags correspond to the destructured props and reflect actual component props to improve editor support and readability.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (14)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css (5)
1-4: LGTM - Modal dialog sizing correctly uses :global for Bootstrap classes.The
:global(.modal-dialog)selector properly targets the Bootstrap modal dialog class that exists outside the CSS module scope.
6-28: Input styles use design tokens consistently.The input states (
:active,:focus-within) properly use CSS custom properties for colors, spacing, and shadows.
30-50: Table styling follows design token conventions.Table header, body cells, rows, and image styles all use design tokens appropriately.
52-76: Member name and link styles use design tokens correctly.The
.membername,.eventsAttended, and.subtleBlueGreyclasses properly usevar(--color-blue-*)tokens for theming.
77-152: Button variants are well-implemented with design tokens.The
.addButton,.removeButton, and.colorPrimaryclasses correctly:
- Use Bootstrap CSS variable overrides (
--bs-btn-*)- Apply design tokens for colors and spacing
- Define hover/active/focus states using
:is()pseudo-classsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css (5)
1-21: Error container styles are well-structured.The error display components use design tokens appropriately. The emoji content in
::beforeis a simple approach for the error icon.
23-26: Verify table header background color intent.The
.tableHeaderuses--color-red-500for background, which creates a red header. Confirm this is intentional for the OrganizationTags table header styling.
36-47: Breadcrumb accessibility pattern is well-implemented.The
::afterpseudo-element technique usingattr(data-text)reserves space for bold text on hover, preventing layout shift. This is a good accessibility practice.Also applies to: 72-85
96-184: Button styles follow established patterns with design tokens.The edit, create, remove, and add button classes properly use:
- Bootstrap CSS variable overrides (
--bs-btn-*)- Design tokens for all colors and spacing
- Consistent state handling with
:is()pseudo-class
221-271: Responsive breakpoints are well-implemented.The media queries at 520px and 1020px provide appropriate mobile and tablet layouts with full-width buttons and stacked containers.
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css (4)
1-85: Container layout duplication is acceptable for this migration phase.The
.btnsContainerstyles are similar to OrganizationTags.module.css. Per the CSS Streamlining Phase 2 approach, some duplication across screens is acceptable; consolidation can happen in a later phase. Based on learnings, this is expected.
87-116: Dropdown styles are well-implemented with comprehensive state handling.The
:is()pseudo-class efficiently combines multiple states. The explicit focus outline on lines 114-116 ensures keyboard accessibility.
118-136: Horizontal rule styling uses design tokens correctly.The negative margin technique for full-width
<hr>and responsive width adjustments are properly implemented with tokens.
138-142: LGTM - Simple scrollable container.The
.list_boxclass provides appropriate overflow handling.
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
Outdated
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css
Outdated
Show resolved
Hide resolved
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
Show resolved
Hide resolved
|
@Piyush-Goenka, thank you for the clarification! I understand now:
Both are reasonable decisions for this incremental refactoring. The PR successfully migrates the three screens with proper CSS modules and design token usage. Approving now! ✅ ✅ Actions performedComments resolved and changes approved. |
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. @Piyush-Goenka, thank you for the confirmation! Just to clarify: I already approved this PR in my previous comment after you explained your design decisions. ✅ The PR is approved and ready to merge. Great work on this CSS refactoring! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx (3)
111-124: Test lacks assertions.This test executes code but has no
expect()statements. Tests without assertions don't verify behavior and won't catch regressions. Add meaningful assertions to validate the expected outcome after clicking the remove button.Suggested fix
it('should render props and text elements test for the page component', async () => { const props = { - toggleRemoveModal: () => true, + toggleRemoveModal: vi.fn(() => true), id: '456', }; renderOrgAdminListCard(props); await wait(); await userEvent.click(screen.getByTestId(/removeAdminBtn/i)); - await wait(2000); + await waitFor(() => { + expect(props.toggleRemoveModal).toHaveBeenCalled(); + }); });
161-172: MissingI18nextProviderwrapper.This inline render lacks the
I18nextProviderwrapper thatrenderOrgAdminListCardincludes. This inconsistency may cause i18n-related warnings or different behavior. Consider reusingrenderOrgAdminListCardor extracting a consistent render helper that accepts custom mocks.Suggested approach
- render( - <MockedProvider link={noDataLink}> - <MemoryRouter initialEntries={['/orgpeople/987']}> - <Routes> - <Route - path="/orgpeople/:orgId" - element={<OrgAdminListCard {...props} />} - /> - </Routes> - </MemoryRouter> - </MockedProvider>, - ); + render( + <MockedProvider link={noDataLink}> + <MemoryRouter initialEntries={['/orgpeople/987']}> + <I18nextProvider i18n={i18nForTest}> + <Routes> + <Route + path="/orgpeople/:orgId" + element={<OrgAdminListCard {...props} />} + /> + </Routes> + </I18nextProvider> + </MemoryRouter> + </MockedProvider>, + );
203-214: Same issue: missingI18nextProviderwrapper.Apply the same fix as suggested for the previous inline render to maintain consistency.
🤖 Fix all issues with AI agents
In @src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx:
- Around line 101-109: The test teardown in the afterEach callback is calling
vi.restoreAllMocks(), which restores mocked implementations and can break
subsequent tests; remove the vi.restoreAllMocks() call and keep
vi.clearAllMocks() so mocks' implementation objects remain intact while call
history is cleared (update the afterEach that currently sets window.location and
calls vi.clearAllMocks()/vi.restoreAllMocks() to only call vi.clearAllMocks()).
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css:
- Around line 5-11: The .errorMessage rule uses a fragile hardcoded margin-top:
25%; replace that with a design token or a robust flex-centering approach:
remove margin-top and either use a space token (e.g. margin-top: var(--space-XX)
or padding-top: var(--space-XX)) consistent with your design tokens, or make the
container full-height (e.g. height: 100% / min-height) and rely on the existing
display:flex with justify-content and align-items to vertically center; update
the .errorMessage CSS accordingly so it uses var(--space-*) or flex centering
instead of 25%.
- Around line 186-198: The .btnsContainer selector is declared twice causing the
first block to be partially overwritten; consolidate both declarations into a
single .btnsContainer rule by merging properties (display, margin, align-items,
gap, flex-shrink, white-space) into one block and remove the duplicate to avoid
conflicting/overwritten styles; ensure the final rule preserves the intended
spacing (include margin and gap) and keep property order consistent with project
style.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
🧰 Additional context used
📓 Path-based instructions (4)
src/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
All components must follow reusable component policy: No inline interfaces, correct portal placement, wrapper usage
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css
🧠 Learnings (12)
📚 Learning: 2025-12-14T17:41:33.572Z
Learnt from: singhaditya73
Repo: PalisadoesFoundation/talawa-admin PR: 0
File: :0-0
Timestamp: 2025-12-14T17:41:33.572Z
Learning: In talawa-admin, using vi.restoreAllMocks() in afterEach blocks destroys hoisted mocks (created with vi.hoisted()) by restoring them to their original implementations, causing "undefined" errors in subsequent tests. Use vi.clearAllMocks() instead, which only clears call history while preserving mock implementations. This is critical for test files like LoginPage.spec.tsx that rely on hoisted mocks (toastMocks, routerMocks, resetReCAPTCHA) across multiple test cases.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-08T16:45:49.201Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6241
File: src/components/AdminPortal/OrgPeopleListCard/OrgPeopleListCard.tsx:12-16
Timestamp: 2026-01-08T16:45:49.201Z
Learning: In TSX components, prefer flat JSDoc parameter documentation format (e.g., param id, param toggleRemoveModal) over the nested format (param props.id). The nested form can break editor parsing. When a component accepts a props object, destructure the props in the function signature to align with the flat JSDoc style (e.g., function MyComp({ id, toggleRemoveModal }: Props) { ... }). Ensure all param tags correspond to the destructured props and reflect actual component props to improve editor support and readability.
Applied to files:
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css
📚 Learning: 2026-01-12T18:08:53.979Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFunds/OrganizationFunds.module.css:12-18
Timestamp: 2026-01-12T18:08:53.979Z
Learning: In CSS migration PRs (e.g., the CSS Streamlining phases), limit the scope to moving styles from global stylesheets to colocated .module.css files and adopting design tokens. Do not attempt layout refactors or general CSS implementation pattern changes unless the PR explicitly states this is the goal. Preserve existing CSS behavior during migration unless a refactor is explicitly intended.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css
📚 Learning: 2026-01-12T12:48:03.589Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFundCampaign/modal/CampaignModal.module.css:1-55
Timestamp: 2026-01-12T12:48:03.589Z
Learning: In CSS streamlining Phase 2, prefer co-located screen-specific CSS modules (e.g., CampaignModal.module.css next to its screen). Some duplication across screens (e.g., modal styles) is acceptable in this phase to complete screen migrations; plan to consolidate shared styles in a later phase and remove duplicates once migration completes. Reviewers should tolerate duplication for this phase but flag excessive duplication and ensure changes are well-documented.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css (1)
1-151: LGTM! Well-structured CSS module following the migration pattern.The module correctly uses design tokens throughout for colors, spacing, typography, and borders. The
rgba()values for box-shadow opacity (e.g., lines 16, 25, 130-132) are acceptable as opacity values are typically not tokenized. The styling covers all necessary states (hover, active, focus, disabled) for interactive elements.Based on learnings, this aligns with the CSS Streamlining Phase 2 approach of colocating styles with components.
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css (1)
1-141: LGTM! Comprehensive responsive styling with proper token usage.The module implements a well-organized responsive layout with appropriate breakpoints (520px, 575.5px, 1020px, 1200px). Design tokens are used consistently for spacing, colors, borders, and shadows. The flex-based layout in
.btnsContainergracefully adapts between screen sizes.Based on learnings, this follows the CSS migration pattern appropriately.
src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css (1)
1-270: Good migration overall with proper token adoption.The module successfully migrates OrganizationTags styles to a colocated CSS module with comprehensive design token usage for colors, spacing, typography, and borders. The responsive breakpoints (520px, 1020px) provide appropriate layout adaptations. The interactive states (hover, focus, active, disabled) are well-covered for buttons and links.
Based on learnings, minor duplication across screens during Phase 2 is acceptable and will be consolidated later.
src/components/AdminPortal/OrgAdminListCard/OrgAdminListCard.spec.tsx (1)
80-98: Good pattern for window.location mocking.Saving the original
window.locationand creating a comprehensive mock object with all properties is a solid approach for test stability. This ensures tests that depend on location properties work consistently.Minor note: Line 93 uses nullish coalescing (
??), which won't trigger the fallback ifpathnameis an empty string. Consider using logical OR (||) if an empty string should also fall back to the default path.
|
There are conflicting files |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsx (2)
64-64: Fix component naming to follow PascalCase convention.The component function is named
organizationVenues(camelCase) but must beOrganizationVenues(PascalCase) to match the file name and follow React component naming conventions per coding guidelines §3.2.🔧 Proposed fix
-function organizationVenues(): JSX.Element { +function OrganizationVenues(): JSX.Element { // Translation hooks for i18n support const { t } = useTranslation('translation', { keyPrefix: 'organizationVenues',-export default organizationVenues; +export default OrganizationVenues;Also applies to: 301-301
64-64: Consider adding TSDoc to the component export.While the file has comprehensive documentation at the top (lines 1-47), per coding guidelines §3.5, exported components should have JSDoc immediately before the function declaration.
📝 Suggested TSDoc addition
+/** + * OrganizationVenues component displays and manages venues for an organization. + * + * Provides search, sort, create, edit, and delete functionality for venues. + * Organization ID is derived from URL parameters. + * + * @returns JSX.Element - The rendered OrganizationVenues component + */ function OrganizationVenues(): JSX.Element { // Translation hooks for i18n supportBased on learnings from the repository, TSDoc format is preferred in TSX files with standard tags like
@paramand@returns.src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (5)
81-81: Movedocument.titleassignment into auseEffect.Setting
document.titledirectly in the function body is a side effect that runs on every render. Wrap it inuseEffectto ensure it only updates when the translation changes.Proposed fix
+ useEffect(() => { + document.title = translateOrgPeople('title'); + }, [translateOrgPeople]); + const [addUserModalisOpen, setAddUserModalIsOpen] = useState(false); - document.title = translateOrgPeople('title');
153-156: Move enum outside the component.
OrganizationMembershipRoleis recreated on every render. Move it outside the component function (before line 75) for better performance and cleaner code organization.Proposed fix
Add before the component function:
enum OrganizationMembershipRole { ADMIN = 'administrator', REGULAR = 'regular', }Then remove lines 153-156 from inside the component.
544-554: Add keyboard accessibility to password toggle.The password visibility toggle uses
onClickonInputGroup.Textbut lacks keyboard support. Users who navigate with keyboards cannot activate this control.Proposed fix
<InputGroup.Text className={`${styles.colorPrimary} ${styles.borderNone} ${styles.colorWhite}`} onClick={togglePassword} data-testid="showPassword" + role="button" + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + togglePassword(); + } + }} + aria-label={showPassword ? tCommon('hidePassword') : tCommon('showPassword')} >Apply the same pattern to the confirm password toggle (lines 570-580).
570-580: Same keyboard accessibility issue on confirm password toggle.Apply the same fix as the password toggle above—add
role="button",tabIndex={0},onKeyDown, andaria-label.
75-598: Test coverage for critical user interactions is incomplete.The test file (1500 lines) covers the
createBtnandcloseBtnbutton clicks with success/error scenarios. However, two significant gaps remain:
Member profile link not tested: The component renders a
Linkcomponent with pathname/member/${currentUrl}/${userDetails.id}in the table rows (around line 290), but no tests verify clicking this link or its navigation behavior.BaseModal backdrop and keyboard props: While modal visibility is tested (e.g.,
addNewUserModalappears/disappears), thebackdrop="static"andkeyboard={false}behavior on the create user modal is not explicitly tested. These prevent accidental dismissal and should be verified.Add tests to cover:
- Clicking the member name link and verifying navigation to the correct member profile path
- Verifying that the create user modal cannot be closed by clicking the backdrop or pressing Escape when
backdrop="static"andkeyboard={false}are set
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/docs/auto-docs/screens/AdminPortal/OrganizationPeople/addMember/AddMember/functions/default.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (3)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Check for disable statements (eslint-disable, istanbul-ignore, it.skip)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (5)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx (1)
34-34: CSS import correctly updated to use colocated module.The import path change from the global stylesheet to
./OrganizationPeople.module.cssaligns with PR objectives. The new CSS module exists with all required styles, and the test file is properly colocated with the component.src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsx (1)
51-51: CSS module migration is properly implemented. The colocated stylesheet exists with all required selectors (.btnsContainer,.dropdown,.mainpageright,.list_box), the component correctly imports and uses the CSS classes via thestylesobject, and the test file is colocated and properly structured.src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (3)
1-73: Imports and TSDoc look good.The TSDoc follows the
@remarksstyle correctly, and the imports properly useBaseModalandAvatarfromshared-componentsinstead of restricted libraries.
430-439: LGTM - Member link routing updated correctly.The updated Link now includes the user ID in the path (
/member/${currentUrl}/${userDetails.id}), which correctly routes to the member detail view.
491-593: Form accessibility improvements look good.The labels are properly associated with inputs via
htmlFor/idpairs, andaria-required="true"is correctly applied to required fields.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (4)
1-42: Move the TSDoc block to the exported component (current comment won’t attach toAddMember).
The doc comment at Lines 1-42 sits above imports, so tooling won’t associate it withAddMember(the exported component). Preferexport default function AddMember()or move the comment immediately above the function. Based on learnings, prefer TSDoc style in TSX.Proposed fix
-/** - * Component: AddMember - * - * This component allows users to add members to an organization. It provides two options: - * 1. Adding an existing user from the user list. - * 2. Creating a new user and adding them to the organization. - * - * ... - * @returns JSX.Element - The rendered AddMember component. - */ import { useLazyQuery, useMutation, useQuery } from '@apollo/client'; ... -// Removed StyledTableCell and StyledTableRow in favor of CSS modules - -function AddMember(): JSX.Element { +/** + * AddMember screen for managing organization members. + * + * @returns JSX.Element + */ +export default function AddMember(): JSX.Element { const { t: translateOrgPeople } = useTranslation('translation', { keyPrefix: 'organizationPeople', }); ... } -export default AddMember;Also applies to: 79-80, 578-578
85-86: Avoid side effects during render (document.title = ...).
Setdocument.titlein auseEffectso it doesn’t run during rendering (and so StrictMode/dev behaviors don’t cause surprises).Proposed fix
- document.title = translateOrgPeople('title'); + useEffect(() => { + document.title = translateOrgPeople('title'); + }, [translateOrgPeople]);
131-133: Remove hardcoded user-facing string in toast interpolation ('Member').
This violates the “no hardcoded strings” rule for UI messaging; make the interpolateditemtranslated too.Proposed fix
NotificationToast.success( - tCommon('addedSuccessfully', { item: 'Member' }) as string, + tCommon('addedSuccessfully', { + item: translateOrgPeople('member'), + }) as string, );
178-189: Guard against null/undefinedcreatedUserIdbefore callingcreateMember.
registeredUser?.data.createUser.user.idcan be missing; callingcreateMember(undefined as any)will break the mutation path and/or toast flow.Proposed fix
const registeredUser = await registerMutation({ variables: { name: createUserVariables.name, email: createUserVariables.email, password: createUserVariables.password, role: OrganizationMembershipRole.REGULAR, isEmailAddressVerified: true, }, }); - const createdUserId = registeredUser?.data.createUser.user.id; - await createMember(createdUserId); + const createdUserId = registeredUser?.data?.createUser?.user?.id; + if (!createdUserId) { + NotificationToast.error( + translateOrgPeople('users.errorCreatingUser') as string, + ); + return; + } + await createMember(createdUserId);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Count-Changed-Files / Checks if number of files changed is acceptable
- GitHub Check: MinIO Compliance Check
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (2)
70-70: Ensure CSS module contract matches usages (class names + file exists).
This import is fine, but it’s now a hard dependency that./AddMember.module.cssexists and exports every referenced key (e.g.,modalContent,tableHeadCell,TableImage, etc.). If any are missing, you’ll get runtime styling regressions and (depending on typing setup) TS errors.
79-284: Colocated test and CSS module verified—no changes needed.The
AddMember.spec.tsxtest file is properly colocated and covers all key flows: modal open/close, user search, pagination, and create-user validations (required fields, password mismatch, error handling). The CSS module (AddMember.module.css) is correctly imported and used throughout the component. Test file contains 22 comprehensive test cases using Vitest + React Testing Library patterns.Typecheck must pass in CI environment with full
node_modulesinstalled.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (1)
162-165: Consider extracting the enum to a shared types file.The
OrganizationMembershipRoleenum is defined inside the component. If this role type is used elsewhere in the codebase, consider moving it to a shared types file for reusability and consistency.🔧 Suggested refactor
Move to
src/types/AdminPortal/OrganizationPeople/interface.tsor a shared enum file:// In a types file export enum OrganizationMembershipRole { ADMIN = 'administrator', REGULAR = 'regular', }Then import where needed.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx (1)
126-146: Save and restore originalwindow.locationto prevent test pollution.The
setupLocationMockfunction replaceswindow.locationbut doesn't preserve the original value. This could cause issues in other tests or test shards. Based on learnings, proper cleanup is important for test isolation.🔧 Proposed fix
+// Store original location +let originalLocation: Location; + // Setup mock window.location const setupLocationMock = () => { + originalLocation = window.location; Object.defineProperty(window, 'location', { value: { href: 'http://localhost/', assign: vi.fn((url) => { const urlObj = new URL(url, 'http://localhost'); window.location.href = urlObj.href; window.location.pathname = urlObj.pathname; window.location.search = urlObj.search; window.location.hash = urlObj.hash; }), reload: vi.fn(), pathname: '/', search: '', hash: '', origin: 'http://localhost', }, - writable: true, + writable: true, + configurable: true, }); }; + +// Restore original location +const restoreLocationMock = () => { + Object.defineProperty(window, 'location', { + value: originalLocation, + writable: true, + configurable: true, + }); +};Then in the test suite:
afterEach(() => { vi.clearAllMocks(); + restoreLocationMock(); });
🤖 Fix all issues with AI agents
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx:
- Around line 121-124: The current useMutation call for
CREATE_ORGANIZATION_MEMBERSHIP_MUTATION_PG uses a string-based refetchQueries
('Organizations') which will match multiple queries; update the mutation options
to refetch using the actual query document instead (e.g., refetchQueries: [{
query: ORGANIZATIONS_LIST }]) or replace with a refetch policy like
refetchPolicy: 'cache-and-network' to reliably update the cache; modify the
useMutation invocation around CREATE_ORGANIZATION_MEMBERSHIP_MUTATION_PG (the
refetchQueries option) accordingly and consider deduplicating operation names
elsewhere if needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧠 Learnings (7)
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx
🧬 Code graph analysis (1)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (1)
src/GraphQl/Mutations/mutations.ts (1)
CREATE_ORGANIZATION_MEMBERSHIP_MUTATION_PG(360-376)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (5)
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (2)
70-70: LGTM - CSS module import correctly colocated.The import change from
'style/app-fixed.module.css'to'./AddMember.module.css'aligns with the PR objective of moving styles from global to component-scoped CSS modules.
137-139: Auto-close modal behavior change is appropriate.The modal now closes automatically after successful member addition. The comment explains this aligns with Cypress test expectations. This improves UX by not requiring manual modal dismissal after a successful action.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.spec.tsx (3)
614-620: LGTM - Test correctly updated to verify auto-close behavior.The test now asserts that
addExistingUserModalis removed from the DOM after successful member addition, which matches the component's new auto-close behavior. UsingqueryByTestIdwithnot.toBeInTheDocument()is the correct pattern for verifying element absence.
372-380: LGTM - Test setup follows best practices.The test setup correctly:
- Uses
vi.clearAllMocks()in bothbeforeEachandafterEach(per learnings for test isolation)- Calls
setupLocationMock()inbeforeEach- Follows the Vitest + React Testing Library framework requirements
32-92: Good approach using force buttons to test pagination guard clauses.The custom TablePagination mock with "Force Next" and "Force Prev" buttons enables testing the early return conditions in
handleChangePagewithout being blocked by disabled states. This is a valid technique for achieving coverage of edge cases.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
Outdated
Show resolved
Hide resolved
Piyush-Goenka
force-pushed
the
refactor/css-pr-08-org-people
branch
from
b54862a to
6ce2df8
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx (1)
30-30: Restricted import violation per coding guidelines §3.4.The
Modalimport fromreact-bootstrapon line 30 violates the no-restricted-imports policy. Per the coding guidelines andeslint.config.js,Modalimports outside of theBaseModalwrapper implementation are not allowed. UseBaseModalfromshared-components/BaseModal/instead.This is a pre-existing violation (35+ files in codebase have the same issue) and represents a broader Modal migration effort. If this file is not being modified in this PR, track the migration in a separate issue. If this file is being actively changed, migrate it to
BaseModalin this PR.
🤖 Fix all issues with AI agents
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css:
- Around line 14-21: The .input:active selector (and similar rules in this
module) uses hardcoded rgba() shadow colors (e.g., rgba(0,0,0,0.3),
rgba(168,199,250,1), rgba(60,64,67,0.15)); replace those literal colors with
design token variables (create tokens like --shadow-color-xs,
--shadow-color-blue, --shadow-color-muted or use existing token names) and
update the box-shadow declarations in the .input:active rule and the other
occurrences noted (around the other box-shadow rules in this file) to reference
those tokens so shadows are driven by design tokens for a future refactor.
- Around line 52-62: The CSS has a duplicated color declaration for .membername
(it's set in the combined selector .eventsAttended, .membername and again in
.membername); remove the redundant color: var(--color-blue-200) from the
.membername block so the color is only declared once in the combined selector,
leaving font-size/font-weight/text-decoration in .membername unchanged.
In @src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css:
- Around line 6-21: The CSS has a redundant color declaration for .membername
(also declared in the combined selector .eventsAttended, .membername); remove
the duplicate color: var(--color-blue-200) from the .membername rule and leave
the shared color in the combined selector, keeping the .membername block’s
font-size, font-weight and text-decoration rules and the .membername:hover rule
unchanged.
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css:
- Around line 133-153: The .inputField rules use hardcoded rgba(...) shadow
colors and an unclear border declaration; update .inputField and
.inputField:focus to use your design system shadow tokens instead of literal
rgba values (replace "box-shadow: 0 var(--shadow-offset-md)
var(--shadow-blur-lg) rgba(0,0,0,0.1)" and the focus shadow with the appropriate
--shadow-* token(s)) and change the focus border from "border: var(--border-0)
solid var(--color-gray-300)" to a clear declaration such as "border: none" or
"border-width: 0 solid var(--color-gray-300)" (or "border-width: 0" with a
separate border-color if needed) so the intent is explicit; keep
.inputField::placeholder unchanged except ensure it uses tokenized color if
available.
- Around line 220-270: The media queries are ordered so the smaller
max-width:520px rules are defined before the larger max-width:1020px rules,
causing the 1020px block to override the 520px styles at narrow viewports; fix
this by swapping the two media query blocks so the max-width:1020px rules appear
before the max-width:520px rules, ensuring .btnsContainer, .btnsContainer
.btnsBlock and .btnsContainer .btnsBlock button/div rules for 520px take
precedence at small screens.
- Around line 182-184: The .addButton:disabled rule only repeats margin-bottom
from the base .addButton and provides no visual feedback; either remove the
.addButton:disabled block or update it to visually indicate disabled state by
adding properties such as opacity: 0.6 (or 0.5), cursor: not-allowed,
pointer-events: none (if appropriate), and muted background/color values to
match your design system; update the selector .addButton:disabled (referencing
the existing .addButton base rule) accordingly so the disabled state is visually
distinct.
- Around line 56-85: The .subTagsLink base rule is declared after its
hover/focus and child icon rules which hurts readability; move the entire
.subTagsLink block (including its nested &::after) so it appears before the
.subTagsLink:hover, .subTagsLink:focus and .subTagsLink i / .subTagsLink:hover i
rules, preserving all declarations (color, font-weight, cursor and the ::after
content) and leaving the hover/focus and visibility rules unchanged so
specificity and behavior remain the same.
- Around line 186-198: Merge the duplicate .btnsContainer rules into a single
declaration: keep display: flex, align-items: center, include margin:
var(--space-9) 0 from the first block and add gap: var(--space-5), flex-shrink:
0, and white-space: nowrap from the second block; remove the redundant duplicate
selector so all properties for .btnsContainer live in one consolidated rule.
In @src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css:
- Around line 104-108: The box-shadow declaration in
OrganizationVenues.module.css uses hardcoded rgba values; extract those
rgba(...) literals into CSS custom properties (e.g., --shadow-color-primary and
--shadow-color-secondary) and replace the inline rgba values in the box-shadow
rule with the new tokens (referencing the existing box-shadow declaration and
its two shadow layers), so the colors can be centralized later while preserving
current offsets/blur/spread semantics.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/style/tokens/colors.css
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsxsrc/style/tokens/colors.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
🧠 Learnings (6)
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/style/tokens/colors.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
📚 Learning: 2026-01-12T18:08:53.979Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFunds/OrganizationFunds.module.css:12-18
Timestamp: 2026-01-12T18:08:53.979Z
Learning: In CSS migration PRs (e.g., the CSS Streamlining phases), limit the scope to moving styles from global stylesheets to colocated .module.css files and adopting design tokens. Do not attempt layout refactors or general CSS implementation pattern changes unless the PR explicitly states this is the goal. Preserve existing CSS behavior during migration unless a refactor is explicitly intended.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/style/tokens/colors.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
📚 Learning: 2026-01-12T12:48:03.589Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFundCampaign/modal/CampaignModal.module.css:1-55
Timestamp: 2026-01-12T12:48:03.589Z
Learning: In CSS streamlining Phase 2, prefer co-located screen-specific CSS modules (e.g., CampaignModal.module.css next to its screen). Some duplication across screens (e.g., modal styles) is acceptable in this phase to complete screen migrations; plan to consolidate shared styles in a later phase and remove duplicates once migration completes. Reviewers should tolerate duplication for this phase but flag excessive duplication and ensure changes are well-documented.
Applied to files:
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.csssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.csssrc/screens/AdminPortal/OrganizationTags/OrganizationTags.module.csssrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsxsrc/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsxsrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsxsrc/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (10)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css (1)
6-43: Note: Style duplication across modules is acceptable for this phase.The
.membername,.subtleBlueGrey, and.removeButtonstyles are duplicated between this file andAddMember.module.css. This duplication is acceptable during CSS Streamlining Phase 2 to complete screen migrations.Plan to consolidate shared styles in a future phase once all screen migrations are complete.
Based on learnings, duplication across screens is tolerated in this migration phase to enable independent screen completion.
src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.tsx (1)
70-70: CSS module migration looks correct.The import path update from the global stylesheet to a colocated
./AddMember.module.cssaligns with the CSS Streamlining Phase 2 objectives. This change properly scopes styles to the component.src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.tsx (1)
34-34: CSS module migration looks correct.The import path update from the global stylesheet to a colocated
./OrganizationPeople.module.cssproperly scopes styles to this component, aligning with the CSS Streamlining Phase 2 objectives.src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css (2)
1-141: CSS module properly uses design tokens.The new colocated CSS module correctly uses design tokens (
var(--space-*),var(--color-*),var(--border-*),var(--shadow-*)) for most values and provides appropriate responsive breakpoints.
30-84: Media query ordering matches the original global stylesheet — no changes needed.Verified that
OrganizationVenues.module.csspreserves the same media query order fromapp-fixed.module.css: 520px query (line 30) followed by 1020px query (line 52). This pattern is consistent throughout the original stylesheet and functions correctly, as the 520px query applies last in the cascade for the smallest viewports, properly overriding the 1020px rules. The migration correctly maintains existing behavior.src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.tsx (1)
51-51: CSS module migration looks correct.The import path update from the global stylesheet to a colocated
./OrganizationVenues.module.cssproperly scopes styles to this component, aligning with the CSS Streamlining Phase 2 objectives.src/screens/AdminPortal/OrganizationTags/OrganizationTags.tsx (1)
37-37: CSS module migration looks correct.The import path update from the global stylesheet to a colocated
./OrganizationTags.module.cssproperly scopes styles to this component, aligning with the CSS Streamlining Phase 2 objectives.src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css (3)
1-21: LGTM — Error state styles use design tokens appropriately.The error container and message styles correctly leverage design tokens for font-size, color, and spacing. The emoji in
::beforeis acceptable for decorative purposes.
23-26: Verify the red table header is intentional.Using
var(--color-red-500)for.tableHeaderbackground is unconventional. If this matches the existing design from the global stylesheet migration, it's fine. Otherwise, consider whether a more neutral color likevar(--color-gray-100)would be more appropriate for table headers.
87-131: LGTM — Scrollable container and button styles are well-implemented.Good use of:
- Design tokens for scrollbar colors and spacing
- Bootstrap CSS variable overrides (
--bs-btn-*) for button customization- Proper state handling with
:hoverand:activepseudo-classes
| .input:active { | ||
| box-shadow: var(--border-shadow-xs) var(--border-shadow-xs) | ||
| var(--border-shadow-xs) rgba(0, 0, 0, 0.3); | ||
|
|
||
| background-color: var(--color-blue-200); | ||
| border-color: var(--color-gray-200); | ||
| color: var(--color-gray-700); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider design tokens for hardcoded shadow colors.
The box-shadow declarations use hardcoded rgba() values (lines 16, 25, 130, 132) instead of design tokens. While this is acceptable for the current CSS migration phase, consider creating color tokens for shadow colors in a future refactor to fully align with the design token system.
Example locations:
- Line 16:
rgba(0, 0, 0, 0.3) - Line 25:
rgba(0, 0, 0, 0.3) - Line 130:
rgba(168, 199, 250, 1) - Line 132:
rgba(60, 64, 67, 0.15)
Based on learnings, this is acceptable for CSS migration PRs focused on moving styles to colocated modules. Full token adoption can be completed in a subsequent phase.
Also applies to: 23-28, 124-134
🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css
around lines 14 - 21, The .input:active selector (and similar rules in this
module) uses hardcoded rgba() shadow colors (e.g., rgba(0,0,0,0.3),
rgba(168,199,250,1), rgba(60,64,67,0.15)); replace those literal colors with
design token variables (create tokens like --shadow-color-xs,
--shadow-color-blue, --shadow-color-muted or use existing token names) and
update the box-shadow declarations in the .input:active rule and the other
occurrences noted (around the other box-shadow rules in this file) to reference
those tokens so shadows are driven by design tokens for a future refactor.
| .eventsAttended, | ||
| .membername { | ||
| color: var(--color-blue-200); | ||
| } | ||
|
|
||
| .membername { | ||
| font-size: var(--font-size-md); | ||
| font-weight: var(--font-weight-bold); | ||
| color: var(--color-blue-200); | ||
| text-decoration: none; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant color declaration in .membername.
The color: var(--color-blue-200) property is declared twice: once in the combined selector (line 54) and again in the .membername individual selector (line 60). This duplication is unnecessary.
♻️ Proposed fix to eliminate redundancy
.eventsAttended,
.membername {
color: var(--color-blue-200);
}
.membername {
font-size: var(--font-size-md);
font-weight: var(--font-weight-bold);
- color: var(--color-blue-200);
text-decoration: none;
}🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationPeople/addMember/AddMember.module.css
around lines 52 - 62, The CSS has a duplicated color declaration for .membername
(it's set in the combined selector .eventsAttended, .membername and again in
.membername); remove the redundant color: var(--color-blue-200) from the
.membername block so the color is only declared once in the combined selector,
leaving font-size/font-weight/text-decoration in .membername unchanged.
| .eventsAttended, | ||
| .membername { | ||
| color: var(--color-blue-200); | ||
| } | ||
|
|
||
| .membername { | ||
| font-size: var(--font-size-md); | ||
| font-weight: var(--font-weight-bold); | ||
| color: var(--color-blue-200); | ||
| text-decoration: none; | ||
| } | ||
|
|
||
| .membername:hover { | ||
| color: var(--color-blue-500); | ||
| text-decoration: underline; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant color declaration in .membername.
The color: var(--color-blue-200) property is declared twice: once in the combined selector (line 8) and again in the .membername individual selector (line 14). This duplication is unnecessary and mirrors the same issue in AddMember.module.css.
♻️ Proposed fix to eliminate redundancy
.eventsAttended,
.membername {
color: var(--color-blue-200);
}
.membername {
font-size: var(--font-size-md);
font-weight: var(--font-weight-bold);
- color: var(--color-blue-200);
text-decoration: none;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .eventsAttended, | |
| .membername { | |
| color: var(--color-blue-200); | |
| } | |
| .membername { | |
| font-size: var(--font-size-md); | |
| font-weight: var(--font-weight-bold); | |
| color: var(--color-blue-200); | |
| text-decoration: none; | |
| } | |
| .membername:hover { | |
| color: var(--color-blue-500); | |
| text-decoration: underline; | |
| } | |
| .eventsAttended, | |
| .membername { | |
| color: var(--color-blue-200); | |
| } | |
| .membername { | |
| font-size: var(--font-size-md); | |
| font-weight: var(--font-weight-bold); | |
| text-decoration: none; | |
| } | |
| .membername:hover { | |
| color: var(--color-blue-500); | |
| text-decoration: underline; | |
| } |
🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.module.css
around lines 6 - 21, The CSS has a redundant color declaration for .membername
(also declared in the combined selector .eventsAttended, .membername); remove
the duplicate color: var(--color-blue-200) from the .membername rule and leave
the shared color in the combined selector, keeping the .membername block’s
font-size, font-weight and text-decoration rules and the .membername:hover rule
unchanged.
| .subTagsLink i { | ||
| visibility: hidden; | ||
| } | ||
|
|
||
| .subTagsLink:hover, | ||
| .subTagsLink:focus { | ||
| color: var(--color-gray-500); | ||
| font-weight: var(--font-weight-semibold); | ||
| text-decoration: underline; | ||
| } | ||
|
|
||
| .subTagsLink:hover i, | ||
| .subTagsLink:focus i { | ||
| visibility: visible; | ||
| } | ||
|
|
||
| .subTagsLink { | ||
| color: var(--color-blue-800); | ||
| font-weight: var(--font-weight-medium); | ||
| cursor: pointer; | ||
|
|
||
| &::after { | ||
| display: block; | ||
| content: attr(data-text); | ||
| font-weight: var(--font-weight-semibold); | ||
| height: 0; | ||
| overflow: hidden; | ||
| visibility: hidden; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reorder selectors: place base .subTagsLink before its state variants.
The base .subTagsLink selector (lines 72-85) is defined after its hover/focus variants (lines 60-65). While CSS specificity makes this work, conventional ordering (base → variants) improves readability.
Suggested reordering
-.subTagsLink i {
- visibility: hidden;
-}
-
-.subTagsLink:hover,
-.subTagsLink:focus {
- color: var(--color-gray-500);
- font-weight: var(--font-weight-semibold);
- text-decoration: underline;
-}
-
-.subTagsLink:hover i,
-.subTagsLink:focus i {
- visibility: visible;
-}
-
.subTagsLink {
color: var(--color-blue-800);
font-weight: var(--font-weight-medium);
cursor: pointer;
&::after {
display: block;
content: attr(data-text);
font-weight: var(--font-weight-semibold);
height: 0;
overflow: hidden;
visibility: hidden;
}
+
+ i {
+ visibility: hidden;
+ }
+
+ &:hover,
+ &:focus {
+ color: var(--color-gray-500);
+ font-weight: var(--font-weight-semibold);
+ text-decoration: underline;
+
+ i {
+ visibility: visible;
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .subTagsLink i { | |
| visibility: hidden; | |
| } | |
| .subTagsLink:hover, | |
| .subTagsLink:focus { | |
| color: var(--color-gray-500); | |
| font-weight: var(--font-weight-semibold); | |
| text-decoration: underline; | |
| } | |
| .subTagsLink:hover i, | |
| .subTagsLink:focus i { | |
| visibility: visible; | |
| } | |
| .subTagsLink { | |
| color: var(--color-blue-800); | |
| font-weight: var(--font-weight-medium); | |
| cursor: pointer; | |
| &::after { | |
| display: block; | |
| content: attr(data-text); | |
| font-weight: var(--font-weight-semibold); | |
| height: 0; | |
| overflow: hidden; | |
| visibility: hidden; | |
| } | |
| } | |
| .subTagsLink { | |
| color: var(--color-blue-800); | |
| font-weight: var(--font-weight-medium); | |
| cursor: pointer; | |
| &::after { | |
| display: block; | |
| content: attr(data-text); | |
| font-weight: var(--font-weight-semibold); | |
| height: 0; | |
| overflow: hidden; | |
| visibility: hidden; | |
| } | |
| i { | |
| visibility: hidden; | |
| } | |
| &:hover, | |
| &:focus { | |
| color: var(--color-gray-500); | |
| font-weight: var(--font-weight-semibold); | |
| text-decoration: underline; | |
| i { | |
| visibility: visible; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css around
lines 56 - 85, The .subTagsLink base rule is declared after its hover/focus and
child icon rules which hurts readability; move the entire .subTagsLink block
(including its nested &::after) so it appears before the .subTagsLink:hover,
.subTagsLink:focus and .subTagsLink i / .subTagsLink:hover i rules, preserving
all declarations (color, font-weight, cursor and the ::after content) and
leaving the hover/focus and visibility rules unchanged so specificity and
behavior remain the same.
| .inputField { | ||
| margin-top: var(--space-4); | ||
| margin-bottom: var(--space-4); | ||
| background-color: var(--color-white); | ||
| border: var(--border-1) solid var(--color-gray-200); | ||
| box-shadow: 0 var(--shadow-offset-md) var(--shadow-blur-lg) rgba(0, 0, 0, 0.1); | ||
| } | ||
|
|
||
| .inputField::placeholder { | ||
| color: var(--color-gray-500); | ||
| opacity: 1; | ||
| } | ||
|
|
||
| .inputField:focus { | ||
| border: var(--border-0) solid var(--color-gray-300); | ||
| background-color: var(--color-gray-50); | ||
| box-shadow: 0 var(--shadow-offset-md) var(--shadow-blur-md) | ||
| rgba(0, 0, 0, 0.25); | ||
| outline: none; | ||
| transition: box-shadow 0.2s ease; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded rgba() values and confusing border-0 declaration.
-
Lines 138, 149-150: Hardcoded
rgba(0, 0, 0, ...)values should ideally use shadow tokens if available in the design system. -
Line 147:
border: var(--border-0) solid var(--color-gray-300)— if--border-0resolves to0, then the color declaration is redundant. Consider usingborder: noneorborder-width: 0for clarity.
Suggested fix for border declaration
.inputField:focus {
- border: var(--border-0) solid var(--color-gray-300);
+ border: none;
background-color: var(--color-gray-50);🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css around
lines 133 - 153, The .inputField rules use hardcoded rgba(...) shadow colors and
an unclear border declaration; update .inputField and .inputField:focus to use
your design system shadow tokens instead of literal rgba values (replace
"box-shadow: 0 var(--shadow-offset-md) var(--shadow-blur-lg) rgba(0,0,0,0.1)"
and the focus shadow with the appropriate --shadow-* token(s)) and change the
focus border from "border: var(--border-0) solid var(--color-gray-300)" to a
clear declaration such as "border: none" or "border-width: 0 solid
var(--color-gray-300)" (or "border-width: 0" with a separate border-color if
needed) so the intent is explicit; keep .inputField::placeholder unchanged
except ensure it uses tokenized color if available.
| .addButton:disabled { | ||
| margin-bottom: var(--space-4); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Incomplete :disabled state for .addButton.
The disabled state only specifies margin-bottom, which is already set in the base .addButton rule (line 170). Consider adding visual differentiation for the disabled state (e.g., reduced opacity, muted colors) or remove this rule if no special styling is needed.
Suggested disabled state styling
.addButton:disabled {
margin-bottom: var(--space-4);
+ opacity: 0.5;
+ cursor: not-allowed;
+ background-color: var(--color-gray-200);
+ border-color: var(--color-gray-200);
}🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css around
lines 182 - 184, The .addButton:disabled rule only repeats margin-bottom from
the base .addButton and provides no visual feedback; either remove the
.addButton:disabled block or update it to visually indicate disabled state by
adding properties such as opacity: 0.6 (or 0.5), cursor: not-allowed,
pointer-events: none (if appropriate), and muted background/color values to
match your design system; update the selector .addButton:disabled (referencing
the existing .addButton base rule) accordingly so the disabled state is visually
distinct.
| .btnsContainer { | ||
| display: flex; | ||
| margin: var(--space-9) 0; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .btnsContainer { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: var(--space-5); | ||
| flex-shrink: 0; | ||
| white-space: nowrap; | ||
| } |
There was a problem hiding this comment.
Duplicate .btnsContainer selector with conflicting properties.
The selector is declared twice:
- Lines 186-190: sets
margin: var(--space-9) 0 - Lines 192-198: omits
margin, addsgap,flex-shrink,white-space
Since the second declaration doesn't include margin, the first declaration's margin will still apply due to CSS cascading. However, this duplication is confusing and error-prone. Merge these into a single declaration.
Suggested merged declaration
.btnsContainer {
display: flex;
- margin: var(--space-9) 0;
align-items: center;
-}
-
-.btnsContainer {
- display: flex;
- align-items: center;
gap: var(--space-5);
flex-shrink: 0;
white-space: nowrap;
+ margin: var(--space-9) 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .btnsContainer { | |
| display: flex; | |
| margin: var(--space-9) 0; | |
| align-items: center; | |
| } | |
| .btnsContainer { | |
| display: flex; | |
| align-items: center; | |
| gap: var(--space-5); | |
| flex-shrink: 0; | |
| white-space: nowrap; | |
| } | |
| .btnsContainer { | |
| display: flex; | |
| align-items: center; | |
| gap: var(--space-5); | |
| flex-shrink: 0; | |
| white-space: nowrap; | |
| margin: var(--space-9) 0; | |
| } |
🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css around
lines 186 - 198, Merge the duplicate .btnsContainer rules into a single
declaration: keep display: flex, align-items: center, include margin:
var(--space-9) 0 from the first block and add gap: var(--space-5), flex-shrink:
0, and white-space: nowrap from the second block; remove the redundant duplicate
selector so all properties for .btnsContainer live in one consolidated rule.
| @media (max-width: 520px) { | ||
| .btnsContainer { | ||
| margin-bottom: 0; | ||
| } | ||
|
|
||
| .btnsContainer .btnsBlock { | ||
| display: block; | ||
| margin-top: var(--space-5); | ||
| margin-right: 0; | ||
| } | ||
|
|
||
| .btnsContainer .btnsBlock div { | ||
| flex: 1; | ||
| } | ||
|
|
||
| .btnsContainer .btnsBlock button { | ||
| margin-bottom: var(--space-5); | ||
| margin-right: 0; | ||
| width: 100%; | ||
| } | ||
| } | ||
|
|
||
| @media (max-width: 1020px) { | ||
| .btnsContainer { | ||
| flex-direction: column; | ||
| margin: var(--space-7) 0; | ||
| } | ||
|
|
||
| .btnsContainer > div { | ||
| width: 100%; | ||
| max-width: 100%; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| .btnsContainer .btnsBlock { | ||
| display: block; | ||
| margin: var(--space-7) 0 0 0; | ||
| justify-content: space-between; | ||
| } | ||
|
|
||
| .btnsContainer .btnsBlock button { | ||
| margin: 0; | ||
| margin-left: 0; | ||
| width: 100%; | ||
| max-width: var(--space-27); | ||
| } | ||
|
|
||
| .btnsContainer .btnsBlock div button { | ||
| margin-right: var(--space-7); | ||
| } | ||
| } |
There was a problem hiding this comment.
Media query order causes smaller breakpoint styles to be overridden.
With max-width queries, the CSS cascade means the last matching rule wins. At viewports ≤520px, both queries apply, but the 1020px rules (lines 242-270) override the 520px rules (lines 220-240) because they appear later.
For example, at 400px viewport width:
- Line 238 sets
.btnsContainer .btnsBlock button { width: 100% } - Line 263-264 then overrides with
width: 100%; max-width: var(--space-27)
Fix: Reorder so larger breakpoints come first when using max-width:
Suggested fix: swap media query order
-@media (max-width: 520px) {
- /* ... 520px rules ... */
-}
-
@media (max-width: 1020px) {
- /* ... 1020px rules ... */
+ /* 1020px rules here */
+}
+
+@media (max-width: 520px) {
+ /* 520px rules here - these now correctly override 1020px rules on small screens */
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @media (max-width: 520px) { | |
| .btnsContainer { | |
| margin-bottom: 0; | |
| } | |
| .btnsContainer .btnsBlock { | |
| display: block; | |
| margin-top: var(--space-5); | |
| margin-right: 0; | |
| } | |
| .btnsContainer .btnsBlock div { | |
| flex: 1; | |
| } | |
| .btnsContainer .btnsBlock button { | |
| margin-bottom: var(--space-5); | |
| margin-right: 0; | |
| width: 100%; | |
| } | |
| } | |
| @media (max-width: 1020px) { | |
| .btnsContainer { | |
| flex-direction: column; | |
| margin: var(--space-7) 0; | |
| } | |
| .btnsContainer > div { | |
| width: 100%; | |
| max-width: 100%; | |
| box-sizing: border-box; | |
| } | |
| .btnsContainer .btnsBlock { | |
| display: block; | |
| margin: var(--space-7) 0 0 0; | |
| justify-content: space-between; | |
| } | |
| .btnsContainer .btnsBlock button { | |
| margin: 0; | |
| margin-left: 0; | |
| width: 100%; | |
| max-width: var(--space-27); | |
| } | |
| .btnsContainer .btnsBlock div button { | |
| margin-right: var(--space-7); | |
| } | |
| } | |
| @media (max-width: 1020px) { | |
| .btnsContainer { | |
| flex-direction: column; | |
| margin: var(--space-7) 0; | |
| } | |
| .btnsContainer > div { | |
| width: 100%; | |
| max-width: 100%; | |
| box-sizing: border-box; | |
| } | |
| .btnsContainer .btnsBlock { | |
| display: block; | |
| margin: var(--space-7) 0 0 0; | |
| justify-content: space-between; | |
| } | |
| .btnsContainer .btnsBlock button { | |
| margin: 0; | |
| margin-left: 0; | |
| width: 100%; | |
| max-width: var(--space-27); | |
| } | |
| .btnsContainer .btnsBlock div button { | |
| margin-right: var(--space-7); | |
| } | |
| } | |
| @media (max-width: 520px) { | |
| .btnsContainer { | |
| margin-bottom: 0; | |
| } | |
| .btnsContainer .btnsBlock { | |
| display: block; | |
| margin-top: var(--space-5); | |
| margin-right: 0; | |
| } | |
| .btnsContainer .btnsBlock div { | |
| flex: 1; | |
| } | |
| .btnsContainer .btnsBlock button { | |
| margin-bottom: var(--space-5); | |
| margin-right: 0; | |
| width: 100%; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationTags/OrganizationTags.module.css around
lines 220 - 270, The media queries are ordered so the smaller max-width:520px
rules are defined before the larger max-width:1020px rules, causing the 1020px
block to override the 520px styles at narrow viewports; fix this by swapping the
two media query blocks so the max-width:1020px rules appear before the
max-width:520px rules, ensuring .btnsContainer, .btnsContainer .btnsBlock and
.btnsContainer .btnsBlock button/div rules for 520px take precedence at small
screens.
| box-shadow: | ||
| 0 var(--shadow-offset-xs) var(--shadow-blur-sm) var(--shadow-spread-none) | ||
| rgba(168, 199, 250, 1), | ||
| 0 var(--shadow-offset-md) var(--shadow-blur-lg) var(--shadow-spread-sm) | ||
| rgba(60, 64, 67, 0.15); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider tokenizing hardcoded rgba values in a future phase.
The box-shadow contains hardcoded rgba values. Per the phased migration approach, this is acceptable for now, but these could be moved to shadow or color tokens in a later consolidation phase for consistency.
box-shadow:
0 var(--shadow-offset-xs) var(--shadow-blur-sm) var(--shadow-spread-none)
rgba(168, 199, 250, 1), /* Consider: var(--shadow-color-primary) */
0 var(--shadow-offset-md) var(--shadow-blur-lg) var(--shadow-spread-sm)
rgba(60, 64, 67, 0.15); /* Consider: var(--shadow-color-secondary) */🤖 Prompt for AI Agents
In @src/screens/AdminPortal/OrganizationVenues/OrganizationVenues.module.css
around lines 104 - 108, The box-shadow declaration in
OrganizationVenues.module.css uses hardcoded rgba values; extract those
rgba(...) literals into CSS custom properties (e.g., --shadow-color-primary and
--shadow-color-secondary) and replace the inline rgba values in the box-shadow
rule with the new tokens (referencing the existing box-shadow declaration and
its two shadow layers), so the colors can be centralized later while preserving
current offsets/blur/spread semantics.
|
@palisadoes |
2 participants
What kind of change does this PR introduce?
This PR fixes four screens to rely on their style sheet rather than a global style sheet.
Issue Number:#5277
Fixes #5277
Summary
This PR introduces changes in the following screens :-
OrganizationPeopleOrganizationTagsOrganizationVenuesThis PR is not the complete PR which solves #5277 but it just solves a part of it and will be raising several other PRs to make this issue complete. Moreover this PR also solves tsdoc errors and import from the shared component.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.