Skip to content

Style teams permissions page#1642

Merged
johbaxter merged 5 commits intodevfrom
1522-task-style-team-permissions-page-in
Aug 14, 2025
Merged

Style teams permissions page#1642
johbaxter merged 5 commits intodevfrom
1522-task-style-team-permissions-page-in

Conversation

@ishumita
Copy link
Copy Markdown
Contributor

@ishumita ishumita commented Aug 1, 2025

Description

Style teams permissions page

Changes Made

Fixed buttons verbiage
Fixed responsive issue
Fixed cards going out of the margins
Fixed description
Fixed styling
Other details are mention in the ticket

Fixed a lot of other linter issues which were creating issues in committing the code.

How to Test

  1. Go to Settings and turn on admin mode
  2. Click team permissions
  3. We can see team permissions page.

Notes

image

@ishumita ishumita requested a review from a team as a code owner August 1, 2025 19:05
@ishumita ishumita linked an issue Aug 1, 2025 that may be closed by this pull request
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

Style teams permissions page


User description

Description

Style teams permissions page

Changes Made

Fixed buttons verbiage
Fixed responsive issue
Fixed cards going out of the margins
Fixed description
Fixed styling
Other details are mention in the ticket

Fixed a lot of other linter issues which were creating issues in committing the code.

How to Test

  1. Go to Settings and turn on admin mode
  2. Click team permissions
  3. We can see team permissions page.

Notes

image

PR Type

Enhancement


Description

  • Refactor TeamTileCard styling and actions menu

  • Optimize TeamsSettingsPage layout with Grid

  • Add debounced search filter and updated hooks

  • Improve modals and type safety


Diagram Walkthrough

flowchart LR
  A["Teams Settings Page"] -- "search/filter" --> B["Filtered Teams"]
  B -- "display cards" --> C["TeamTileCard"]
  C -- "open menu" --> M["Action Menu"]
  M -- "add member" --> D["Add Members Modal"]
  M -- "edit team" --> E["Edit Team Modal"]
  M -- "delete team" --> F["Delete Confirmation"]
Loading

File Walkthrough

Relevant files
Enhancement
TeamTileCard.tsx
Refactor TeamTileCard styling and actions                               

packages/client/src/components/teams/TeamTileCard.tsx

  • Reorganized imports and formatting cleanup
  • Added NonCredentialedUser type and useCallback hooks
  • Switched Popover to Menu for action menu
  • Enhanced styled components (WebkitLineClamp, width, modal styling)
+761/-735
TeamsSettingsPage.tsx
Optimize TeamsSettingsPage layout and search                         

packages/client/src/pages/settings/TeamsSettingsPage.tsx

  • Reformatted imports and styled components
  • Replaced custom grid with MUI Grid container
  • Added debounced filterTeams search
  • Updated AddTeamModal behavior and sorting menu
+281/-269

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /review

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Tag Rendering

The .map over tag only returns a component when i <= 3, potentially producing undefined entries for higher indices. Consider slicing the array or filtering before mapping to avoid rendering undefined elements.

{tag !== undefined &&
	(Array.isArray(tag) ? (
		tag.map((t, i) => {
			if (i <= 3) {
				return (
					<StyledTagChip
						maxWidth={
							tag.length === 2
								? "100px"
								: tag.length === 1
									? "200px"
									: "75px"
						}
						key={`${id}${i}`}
						label={t}
						variant="filled"
					/>
				);
			}
		})
	) : (
Missing Dependencies

The useEffect watching isScrollBottom updates offset by calling getAdditionalUsersNonGroup, but offset is not included in its dependency list. This can lead to stale state and missed loads. Ensure all used state and functions are declared in dependencies.

useEffect(() => {
	if (isScrollBottom) {
		if (canCollect) {
			getAdditionalUsersNonGroup();
		}
	}
}, [isScrollBottom, canCollect]);
Sort Detection

The isAsc and isDesc helpers compare filteredTeams via JSON.stringify on object arrays, which can be expensive and brittle. Consider tracking sort order in state or comparing only IDs to improve reliability and performance.

const isAsc = () => {
	const sorted = [...filteredTeams].sort((a, b) => {
		return a.id.localeCompare(b.id);
	});
	return JSON.stringify(filteredTeams) === JSON.stringify(sorted);
};

const isDesc = () => {
	const sorted = [...filteredTeams].sort((a, b) => {
		return b.id.localeCompare(a.id);
	});
	return JSON.stringify(filteredTeams) === JSON.stringify(sorted);
};

@QodoAI-Agent
Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
General
Close modal after all adds

Closing the modal and clearing selected users inside the loop means it happens after
the first successful add, preventing subsequent operations from reflecting in UI.
Move those calls outside the loop so they run only once after all requests succeed.

packages/client/src/components/teams/TeamTileCard.tsx [350-378]

 for (let i = 0; i < requests.length; i++) {
 	const response: AxiosResponse<{ success: boolean }> | null =
 		await monolithStore.addTeamUser(
 			id,
 			requests[i].type,
 			requests[i].userid,
 			true,
 		);
-
-	if (!response) {
+	if (!response || !response.data.success) {
+		notification.add({
+			color: "error",
+			message: "Error changing user permissions",
+		});
 		return;
 	}
+}
+// after all requests succeed
+setAddMembersModal(false);
+setSelectedNonCredentialedUsers([]);
+notification.add({
+	color: "success",
+	message: "Successfully added member permissions",
+});
 
-	// ignore if there is no response
-	if (response) {
-		setAddMembersModal(false);
-		setSelectedNonCredentialedUsers([]);
-
-		notification.add({
-			color: "success",
-			message: "Successfully added member permissions",
-		});
-	} else {
-		notification.add({
-			color: "error",
-			message: `Error changing user permissions`,
-		});
-	}
-}
-
Suggestion importance[1-10]: 7

__

Why: Clearing the modal and selections inside the loop only runs on the first iteration, so moving them after the loop ensures the UI updates once all requests complete.

Medium
Expand user interface fields

The NonCredentialedUser interface only declares name but in practice your code uses
properties like id, type, email, and color. Expand the interface to match the actual
shape returned by getNonTeamUsers to maintain type safety and prevent errors.

packages/client/src/components/teams/TeamTileCard.tsx [45-47]

 interface NonCredentialedUser {
+	id: string;
 	name: string;
+	type: string;
+	email: string;
+	color: string;
 }
Suggestion importance[1-10]: 5

__

Why: The interface NonCredentialedUser only defines name but the code uses id, type, email, and color, so extending it prevents type mismatches.

Low
Possible issue
Reset loading on non-CUSTOM branch

If type is not "CUSTOM", the loading states are never reset, causing isLoading (and
possibly searchLoading) to remain true indefinitely. Add an else or a finally block
to clear these flags when the branch is skipped.

packages/client/src/components/teams/TeamTileCard.tsx [208-262]

 const getUsersNonGroup = useCallback(
 	async (reset: boolean) => {
 		if (isLoading) {
 			return;
 		}
 		setIsLoading(true);
 
 		if (type === "CUSTOM") {
 			try {
 				const response = await monolithStore.getNonTeamUsers(
 					id,
 					AUTOCOMPLETE_LIMIT,
 					offset,
 					searchMemberInput,
 				);
-				// ignore if there is no response
 				if (response) {
 					...
-					setIsLoading(false);
-					setSearchLoading(false);
 				}
 			} catch (e) {
 				notification.add({ ... });
+			} finally {
 				setIsLoading(false);
 				setSearchLoading(false);
 			}
+		} else {
+			setIsLoading(false);
+			setSearchLoading(false);
 		}
 	},
 	[isLoading, type, id, offset, searchMemberInput, nonCredentialedUsers, monolithStore, notification],
 );
Suggestion importance[1-10]: 7

__

Why: Without an else or finally for the type !== "CUSTOM" branch, isLoading and searchLoading stay true, causing the loading state to hang indefinitely.

Medium

@ishumita ishumita requested a review from AAfghahi August 1, 2025 19:28
onClick={() => onClick(id)}
bordercolor={randomColor}
>
{/* Use Card.Media instead, uses img tag */}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abt reason this is in here? WHy not use Card.media?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is some outdated comment. Removed it.

const { adminMode } = useSettings();
const { monolithStore } = useRootStore();
const navigate = useNavigate();
const { monolithStore } = useRootStore();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we deleted admin mode? I suppose that is part of what Kunal said, is that in the original ticket? Did @themaherkhalil sign off on that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching.
I don't know, why earlier linter was showing me the error that 'admin mode and useSettings() hook' not used. So I removed it. Added it back.

Copy link
Copy Markdown
Contributor

@johbaxter johbaxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you go through tickets, please go back and fix code standards

image

@johbaxter johbaxter merged commit 124a267 into dev Aug 14, 2025
3 checks passed
@johbaxter johbaxter deleted the 1522-task-style-team-permissions-page-in branch August 14, 2025 16:27
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Changelog updates: 🔄

2025-08-14 *

Changed

  • Updated styling and layout for the Teams permissions page, including improved cards, menus, and responsive behavior.

Fixed

  • Resolved layout overflow, responsiveness, and minor UI issues; addressed side effects causing unnecessary re-renders.

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TASK] Style "Team Permissions" landing page in Settings

4 participants