Skip to content

Fix #4152, use separate state for collection info panel and collection selection dialog#4155

Merged
marlo-longley merged 8 commits intomainfrom
fix-4152
Jul 17, 2025
Merged

Fix #4152, use separate state for collection info panel and collection selection dialog#4155
marlo-longley merged 8 commits intomainfrom
fix-4152

Conversation

@lutzhelm
Copy link
Contributor

Replacement for PR #4154; addresses problems mentioned in this comment.

@codecov
Copy link

codecov bot commented Jun 18, 2025

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.12%. Comparing base (92f1c08) to head (3eb64ea).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/components/CollectionDialog.jsx 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4155      +/-   ##
==========================================
+ Coverage   94.91%   95.12%   +0.21%     
==========================================
  Files         325      325              
  Lines       16152    16153       +1     
  Branches     2535     2547      +12     
==========================================
+ Hits        15331    15366      +35     
+ Misses        816      782      -34     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lutzhelm lutzhelm marked this pull request as draft June 18, 2025 14:04
});
} else {
addWindow({ collectionPath: [...collectionPath, manifestId], manifestId: m.id });
addWindow({ collectionPath: [...dialogCollectionPath, manifestId], manifestId: m.id });
Copy link
Contributor Author

@lutzhelm lutzhelm Jun 25, 2025

Choose a reason for hiding this comment

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

I'm going to leave this line untested. It does not actually make a lot of sense that there would be no windowId. The only way this line is ever executed would be a plugin that reuses this component. Might even remove this path it in a separate PR.

@lutzhelm lutzhelm marked this pull request as ready for review July 3, 2025 10:30
@lutzhelm
Copy link
Contributor Author

lutzhelm commented Jul 3, 2025

Commit 5112946 has been cherry picked from #4159.

marlo-longley
marlo-longley previously approved these changes Jul 3, 2025
Copy link
Member

@marlo-longley marlo-longley left a comment

Choose a reason for hiding this comment

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

This works for me, @lutzhelm. It addresses the issue found in PR #4154

@marlo-longley marlo-longley merged commit 22953d1 into main Jul 17, 2025
8 of 9 checks passed
@marlo-longley marlo-longley deleted the fix-4152 branch July 17, 2025 13:43
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.

3 participants