Skip to content

feat: styling for add engines modal#1623

Merged
johbaxter merged 2 commits intodevfrom
style_add_engines
Aug 1, 2025
Merged

feat: styling for add engines modal#1623
johbaxter merged 2 commits intodevfrom
style_add_engines

Conversation

@ishumita
Copy link
Copy Markdown
Contributor

Description

This PR has changes for styling Add Engines Modal

Changes Made

All the changes are made in TeamEnginesTable.tsx
Changed some texts in the modal to match figma.
Fixed styling of the images
Styling of the selected projects is off, see the Figma but the spacing is off and it should also indicate what kind of engine it is Added Images for Engines
Added type of engines
All other changes are mentioned here.

How to Test

  1. Go to team permissions page
  2. Select any team card
  3. Click on Add Engines
  4. Modal will open where we have all the changes
image

@ishumita ishumita requested a review from a team as a code owner July 30, 2025 21:40
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@ishumita
Copy link
Copy Markdown
Contributor Author

There were a lot of linter warning due to which code commit was not happening. Had to fix those issues too.

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat: styling for add engines modal


User description

Description

This PR has changes for styling Add Engines Modal

Changes Made

All the changes are made in TeamEnginesTable.tsx
Changed some texts in the modal to match figma.
Fixed styling of the images
Styling of the selected projects is off, see the Figma but the spacing is off and it should also indicate what kind of engine it is Added Images for Engines
Added type of engines
All other changes are mentioned here.

How to Test

  1. Go to team permissions page
  2. Select any team card
  3. Click on Add Engines
  4. Modal will open where we have all the changes
image

PR Type

Enhancement


Description

  • Cleaned up imports and reorder modules

  • Added random project images mapping logic

  • Restyled Add Engines modal UI components

  • Refactored state hooks with TypeScript types


Diagram Walkthrough

flowchart LR
  A["TeamEnginesTable"] --> B["StyledModal (Add Engines)"]
  B --> C["getRandomImageForProject()"]
  C --> D["Engine Avatar List"]
  D --> E["Permissions Selection"]
  E --> F["submitNonGroupEngines()"]
Loading

File Walkthrough

Relevant files
Enhancement
TeamEnginesTable.tsx
Refactor & style TeamEnginesTable component                           

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

  • Reorganized imports to MUI icons and @semoss/ui
  • Introduced projectImages array and mapping hook
  • Updated modal typography, avatars, cards, and buttons
  • Refactored component props and state interfaces
+1405/-1264

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /review

@ishumita ishumita requested a review from AAfghahi July 30, 2025 21:41
@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

Image Mapping Leak

The getRandomImageForProject hook uses projectImageMap but never resets it when engines change or modal closes, potentially causing stale image associations or memory bloat.

const [projectImageMap, setProjectImageMap] = useState<
	Record<string, string>
>({});

/**
 * @name getEngines
 * @desc Gets all engines without credentials
 */
const getEngines = async (reset: boolean) => {
	if (isLoading) {
		return;
	}
	setIsLoading(true);
	try {
		let response;
		// possibly add more db table columns / keys here to get id type for display under engines
		// eslint-disable-next-line prefer-const
		response = await monolithStore.getUnassignedTeamEngines(
			groupId,
			groupType,
			AUTOCOMPLETE_LIMIT,
			offset,
			searchEngineInput,
		);

		// ignore if there is no response
		if (response) {
			let requests = reset ? [] : nonCredentialedEngines;
			const engines = response.map((val: Engine) => {
				return {
					...val,
					color: colors[
						Math.floor(Math.random() * colors.length)
					],
				};
			});

			requests = requests.concat(engines);
			setNonCredentialedEngines(requests);
			setCanCollect(engines.length === AUTOCOMPLETE_LIMIT);
			setIsLoading(false);
			setSearchLoading(false);
		}
	} catch (e) {
		notification.add({
			color: "error",
			message: String(e),
		});
		setIsLoading(false);
		setSearchLoading(false);
	}
};

const getRandomImageForProject = useCallback(
	(projectId: string) => {
		if (projectImageMap[projectId]) {
			return projectImageMap[projectId];
		}
		const randomIndex = Math.floor(
			Math.random() * projectImages.length,
		);
		const newImage = projectImages[randomIndex];

		// Only update state if we don't have an image for this project
		if (!projectImageMap[projectId]) {
			setProjectImageMap((prev) => ({
				...prev,
				[projectId]: newImage,
			}));
		}

		return newImage;
	},
	[projectImageMap],
);
Scroll Detection

The nearBottom calculation subtracts 25 before comparing to clientHeight, which may misdetect bottom-of-list and repeatedly trigger fetches, impacting performance.

const nearBottom = (
	target: {
		scrollHeight?: number;
		scrollTop?: number;
		clientHeight?: number;
	} = {},
) => {
	const diff = Math.round(target.scrollHeight - target.scrollTop);
	return diff - 25 <= target.clientHeight;
};
Effect Dependencies

The useEffect fetching team engines references monolithStore.getTeamEngines but omits monolithStore itself in its dependency array, risking stale calls or missed updates.

useEffect(() => {
	monolithStore
		.getTeamEngines(
			groupId,
			groupType,
			limit,
			enginesPage * limit - limit, // offset
			searchFilter,
		)
		.then((data) => {
			setEngines(data);
			setHasEngines(true);
		});
}, [

@ishumita ishumita linked an issue Jul 30, 2025 that may be closed by this pull request
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Normalize engine identifiers

Include a normalized engineid field when mapping unassigned engines so that UI
references to engine.engineid work correctly. Map engineid from the returned
engine_id. This ensures the table keys and selection logic function as intended.

packages/client/src/components/teams/TeamEnginesTable.tsx [287-293]

 const engines = response.map((val: Engine) => {
   return {
     ...val,
+    engineid: val.engine_id,
     color: colors[Math.floor(Math.random() * colors.length)],
   };
 });
Suggestion importance[1-10]: 8

__

Why: Adding engineid: val.engine_id ensures the mapped objects always include the correct identifier, preventing missing keys and fixing table row selection logic that relies on engineid.

Medium
General
Use unique engine IDs for images

Use the unique engine_id rather than engine_name when fetching a random image to
avoid collisions if two engines share the same name. Passing engine.engine_id
ensures each engine consistently maps to its own image.

packages/client/src/components/teams/TeamEnginesTable.tsx [1047-1049]

-src={getRandomImageForProject(engine.engine_name)}
+src={getRandomImageForProject(engine.engine_id)}
Suggestion importance[1-10]: 5

__

Why: Switching to engine_id guarantees a unique key for getRandomImageForProject, avoiding collisions when two engines share the same name and improving consistency in image mapping.

Low

@johbaxter johbaxter merged commit 88452a5 into dev Aug 1, 2025
4 checks passed
@johbaxter johbaxter deleted the style_add_engines branch August 1, 2025 18:36
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Changelog updates: 🔄

2025-08-01 #1623

Changed

  • Refined styling and layout for the Add Engines modal, including updated text and image spacing.

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 "Add Engines" Modal in Details Page

4 participants