Do not show used and latest pack info if not available#266
Open
edriouk wants to merge 3 commits into
Open
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
2 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the “Pack Properties” dialog in the Manage Components/Packs webview to avoid displaying “Used Pack” and “Latest Installed Pack” details when the selected pack is not installed/available (Issue #255).
Changes:
- Introduces
packDisplayNameto suppress rendering pack identifiers when the pack is reported as “Pack not installed”. - Disables the “Update” action when there is no installed/latest pack to operate on.
- Adds unit tests covering the new “hide when not installed” behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/views/manage-components-packs/view/components/pack-properties.tsx | Adjusts displayed pack identifiers and update-button enablement when pack info isn’t available. |
| src/views/manage-components-packs/view/components/pack-properties.test.tsx | Adds tests intended to validate hiding/displaying “Used Pack” under different installation states. |
Comments suppressed due to low confidence (2)
src/views/manage-components-packs/view/components/pack-properties.tsx:125
hasNewerOnlineVersionbecomes true whenlatestInstalledPackis empty/undefined (because''.endsWith(...)is false). That makes the “versions” button show the warning color even when there is no installed version to compare against, which is inconsistent with the meaning of “newer online version”. Gate this comparison onlatestInstalledPackbeing truthy (or track installed version separately) so the warning state only appears when an installed version exists.
const latestInstalledPack = latestUpgradable ? `${pack?.name}@${latestUpgradable}` : packDisplayName;
const hasNewerOnlineVersion = !!pack?.latestOnlineVersion && !latestInstalledPack?.endsWith(pack.latestOnlineVersion);
const onlineTooltip = hasNewerOnlineVersion ? <div>Latest version available online: {pack.latestOnlineVersion}</div> : undefined;
src/views/manage-components-packs/view/components/pack-properties.test.tsx:387
- The “passes ... to PackTitleLink” tests only check that the cell
textContentis empty/non-empty, butPackTitleLinkalways renders an external-link button even whenpackNameis empty. This means the test can pass while still showing an unlabeled clickable icon in the UI. Consider asserting on the presence/absence of the link button (e.g.,getByRole('link', { name: 'Open pack URL' })) and/or that the row is not rendered at all when the pack isn’t installed.
it('passes empty packName to PackTitleLink when description is "Pack not installed" and openFile is provided', () => {
const pack = createMockPack({ description: 'Pack not installed' });
const mockOpenFile = jest.fn();
render(
<PackPropertiesDialog
pack={pack}
state={{ unlilnkRequestStack: [], selectedTargetType: selectedTargetType }}
allOrigins={createMockAllOrigins()}
openFile={mockOpenFile}
onClose={mockOnClose}
/>
);
const table = screen.getByText('Used Pack:').closest('table');
expect(table).toBeDefined();
if (table) {
const cells = table.querySelectorAll('td');
const usedPackCell = Array.from(cells).find(cell => cell.textContent === 'Used Pack:');
if (usedPackCell) {
const valueCell = usedPackCell.nextElementSibling as HTMLElement;
// PackTitleLink with empty packName should not display visible text
expect(valueCell?.textContent?.trim()).toBe('');
}
}
});
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (1)
🛟 Help
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| /> | ||
| ); | ||
|
|
||
| expect(screen.queryByText('Used Pack:')).toBeNull(); |
| /> | ||
| ); | ||
|
|
||
| expect(screen.getAllByText('ARM::CMSIS@6.1.0')).toBeDefined(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
Changes
Screenshots
Checklist