Fix marketplace filter select width#6544
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the styling of select dropdowns and filter controls in the Marketplace view, adding ellipsis overflow handling and changing the width of several filter components from 120px to 160px. The feedback suggests moving the getSelectStyles helper function outside of the component to prevent unnecessary recreations on render, and refactoring the hardcoded width of 160px into a shared constant or referencing an existing configuration property to avoid duplication.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
|
|
||
| const getSelectStyles = (borderColor, isDarkMode) => ({ | ||
| width: '100%', |
There was a problem hiding this comment.
The helper function getSelectStyles is defined inside the Marketplace component but does not depend on any component state or props (it only uses its parameters borderColor and isDarkMode). Defining it inside the component causes it to be recreated on every render. Moving it outside the Marketplace component (e.g., right below the MenuProps constant) is a more efficient and maintainable approach.
There was a problem hiding this comment.
Thanks, fixed in a26439b. I moved getSelectStyles out of the component so it is not recreated on each render.
| flexDirection: 'column', | ||
| justifyContent: 'end', | ||
| minWidth: 120 | ||
| width: 160 |
There was a problem hiding this comment.
To avoid duplicating the magic number 160 across multiple FormControl components, you can reference the existing width defined in MenuProps.PaperProps.style.width. Alternatively, you can define a shared constant at the top of the file (e.g., const FILTER_WIDTH = 160) and use it consistently.
| width: 160 | |
| width: MenuProps.PaperProps.style.width |
There was a problem hiding this comment.
Thanks, fixed in a26439b. The filter controls now reuse MenuProps.PaperProps.style.width instead of repeating the literal width.
| flexDirection: 'column', | ||
| justifyContent: 'end', | ||
| minWidth: 120 | ||
| width: 160 |
There was a problem hiding this comment.
To avoid duplicating the magic number 160 across multiple FormControl components, you can reference the existing width defined in MenuProps.PaperProps.style.width. Alternatively, you can define a shared constant at the top of the file (e.g., const FILTER_WIDTH = 160) and use it consistently.
| width: 160 | |
| width: MenuProps.PaperProps.style.width |
There was a problem hiding this comment.
Thanks, fixed in a26439b. The filter controls now reuse MenuProps.PaperProps.style.width instead of repeating the literal width.
| flexDirection: 'column', | ||
| justifyContent: 'end', | ||
| minWidth: 120 | ||
| width: 160 |
There was a problem hiding this comment.
To avoid duplicating the magic number 160 across multiple FormControl components, you can reference the existing width defined in MenuProps.PaperProps.style.width. Alternatively, you can define a shared constant at the top of the file (e.g., const FILTER_WIDTH = 160) and use it consistently.
| width: 160 | |
| width: MenuProps.PaperProps.style.width |
There was a problem hiding this comment.
Thanks, fixed in a26439b. The filter controls now reuse MenuProps.PaperProps.style.width instead of repeating the literal width.
Summary
Tests
Notes