Skip to content

feat: adding platform to scan card + some refactoring#1650

Merged
netomi merged 3 commits into
eclipse-openvsx:masterfrom
gnugomez:gnugomez/master/8201
Mar 2, 2026
Merged

feat: adding platform to scan card + some refactoring#1650
netomi merged 3 commits into
eclipse-openvsx:masterfrom
gnugomez:gnugomez/master/8201

Conversation

@gnugomez

@gnugomez gnugomez commented Feb 27, 2026

Copy link
Copy Markdown
Member

@gnugomez

Copy link
Copy Markdown
Member Author

I’m using some styled versions of the components to avoid having duplicated styles throughout this component, and I’m also breaking it down into several smaller components.

We could potentially move them to other files, but I think that for now having them here is fine. Now the main component sits at the end of the file and it’s far more clear what the purpose of it is.

@autumnfound autumnfound left a comment

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.

Overall LGTM

sx={{ display: 'flex', alignItems: 'center', justifyContent: 'flex-end' }}
>
{showCheckbox && (
<SelectionCheckbox

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.

There doesn't seem to be a legend for this, and it doesn't seem super obvious what this would do without the code as context. Could I see a screenshot of this to get a better picture of how this appears in the scan card?

onChange?: (checked: boolean) => void;
}> = ({ checked = false, onChange }) => {
const theme = useTheme();
const [isHovering, setIsHovering] = useState(false);

@oliviergoulet5 oliviergoulet5 Feb 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

question: You can't achieve hover styles with MUI's styled without using React state? I'm probably missing some context here, but it would be nice not to rely on React logic + event handlers for hover styles.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see this was introduced before this PR. Nevermind if this is out of scope.

…arams for extension admin page, fix lint errors, add changelog
@netomi netomi self-requested a review March 2, 2026 09:28

@netomi netomi left a comment

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.

LGTM,

I did some minor changes:

  • add links to the extension and extension admin page in the scan card for convenience
  • fix lint error that were unrelated
  • add changelog entry

@netomi netomi merged commit 05f03ff into eclipse-openvsx:master Mar 2, 2026
4 checks passed
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.

Adjust ExtensionScan card with useful information

4 participants