Skip to content

Commit 6482092

Browse files
mcmiremcmire
committed
Improve UI/copy for per-package error messages
When the user chooses to include a package in the release, they may be asked to also include dependencies or dependents of that package. The UI for these error messages, as well as the messages themselves, is not as user-friendly as it could be. This commit adds an expandable "help text" for each kind of error to explain what actions the user should take and why.
1 parent a7f83c4 commit 6482092

File tree

2 files changed

+107
-42
lines changed

2 files changed

+107
-42
lines changed

‎src/ui/DependencyErrorSection.tsx‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ type DependencyErrorSectionProps = {
22
title: string;
33
items: string[];
44
setSelections: React.Dispatch<React.SetStateAction<Record<string, string>>>;
5-
description: string;
5+
errorSubject: string;
6+
errorDetails: React.ReactNode;
67
};
78

89
/**
@@ -12,19 +13,22 @@ type DependencyErrorSectionProps = {
1213
* @param props.title - The title of the section.
1314
* @param props.items - The missing dependents or dependencies.
1415
* @param props.setSelections - Updates data around packages selected for the release.
15-
* @param props.description - Describes the error.
16+
* @param props.errorSubject - Summarizes the error.
17+
* @param props.errorDetails - Explains more about the error and how the user
18+
* can fix it.
1619
* @returns The section component.
1720
*/
1821
export function DependencyErrorSection({
1922
title,
2023
items,
2124
setSelections,
22-
description,
25+
errorSubject,
26+
errorDetails,
2327
}: DependencyErrorSectionProps) {
2428
return (
25-
<div className="text-red-800">
29+
<div className="mt-4 pt-4 border-t border-red-200">
2630
<div className="flex justify-between items-center mb-2">
27-
<p className="font-semibold">{title}:</p>
31+
<p className="text-lg font-semibold text-red-700">{title}</p>
2832
<button
2933
onClick={() => {
3034
setSelections((prev) => ({
@@ -43,20 +47,31 @@ export function DependencyErrorSection({
4347
Skip All
4448
</button>
4549
</div>
46-
<p className="text-sm mb-2">{description}</p>
47-
<ul className="list-disc ml-4">
50+
<p className="mb-2">{errorSubject}</p>
51+
<details className="bg-blue-50 border border-blue-200 rounded p-2 my-4 w-fit text-sm [&:open]:w-auto">
52+
<summary className="text-blue-800 font-semibold hover:underline cursor-pointer">
53+
Read more
54+
</summary>
55+
<div className="mt-2 ml-4">{errorDetails}</div>
56+
</details>
57+
<ul className="list-disc">
4858
{items.map((dep) => (
49-
<li key={dep} className="flex justify-between items-center mb-2">
50-
<span
51-
onClick={() => {
59+
<li
60+
key={dep}
61+
className="flex justify-between items-center mb-2 text-gray-700"
62+
>
63+
<a
64+
href="#"
65+
onClick={(event) => {
66+
event.preventDefault();
5267
document
5368
.getElementById(`package-${dep}`)
5469
?.scrollIntoView({ behavior: 'smooth' });
5570
}}
5671
className="cursor-pointer hover:underline"
5772
>
5873
{dep}
59-
</span>
74+
</a>
6075
<button
6176
onClick={() =>
6277
setSelections((prev) => ({

‎src/ui/PackageItem.tsx‎

Lines changed: 81 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ export function PackageItem({
9191
}`}
9292
>
9393
<div className="flex items-start gap-3">
94-
<div className="pt-1">
95-
{showCheckbox && (
94+
{showCheckbox && (
95+
<div className="pt-1">
9696
<input
9797
type="checkbox"
9898
checked={isSelected}
@@ -101,8 +101,8 @@ export function PackageItem({
101101
${isSelected ? 'text-blue-600' : 'text-gray-300'}
102102
`}
103103
/>
104-
)}
105-
</div>
104+
</div>
105+
)}
106106
<div className="flex-1">
107107
<h2 className="text-xl font-semibold">{pkg.name}</h2>
108108
<div className="flex items-center justify-between">
@@ -145,33 +145,83 @@ export function PackageItem({
145145
</div>
146146

147147
{packageDependencyErrors[pkg.name] && (
148-
<div className="mt-2 p-3 bg-red-50 border border-red-200 rounded">
149-
<div className="flex-grow">
150-
{packageDependencyErrors[pkg.name].missingDependencies.length >
151-
0 && (
152-
<DependencyErrorSection
153-
title="Missing Dependencies"
154-
items={packageDependencyErrors[pkg.name].missingDependencies}
155-
setSelections={setSelections}
156-
description={`The following packages are dependencies or peer dependencies of ${pkg.name}. Because they may have introduced new changes that ${pkg.name} is now using, you need to verify whether to include them in the release.
157-
158-
To do this, look at the change history for each package and compare it with the change history for ${pkg.name}. If ${pkg.name} uses any new changes from a package, then you need to include it by bumping its version. If you have confirmed that the changes to a package do not affect ${pkg.name}, you may omit it from the release by choosing "Skip" instead.`}
159-
/>
160-
)}
161-
{packageDependencyErrors[pkg.name].missingDependentNames.length >
162-
0 && (
163-
<div className="mt-4">
164-
<DependencyErrorSection
165-
title="Missing Dependents"
166-
items={
167-
packageDependencyErrors[pkg.name].missingDependentNames
168-
}
169-
setSelections={setSelections}
170-
description={`Because ${pkg.name} is being released with a new major version, to prevent peer dependency warnings in consuming projects, all of the following packages which list ${pkg.name} as a peer dependency need to be included in the release. Please choose new versions for these packages. If for some reason you feel it is safe to omit a package you may choose "Skip".`}
171-
/>
172-
</div>
173-
)}
174-
</div>
148+
<div className="flex-grow flex flex-col gap-2">
149+
{packageDependencyErrors[pkg.name].missingDependencies.length > 0 && (
150+
<DependencyErrorSection
151+
title="Missing Dependencies"
152+
items={packageDependencyErrors[pkg.name].missingDependencies}
153+
setSelections={setSelections}
154+
errorSubject={`You've included ${pkg.name} in the release. However, this package has direct or peer dependencies which have unreleased changes, and you may need to include them as well.`}
155+
errorDetails={
156+
<>
157+
<p className="mb-2">
158+
To resolve these errors, you need to look the changelog or
159+
commit history for {pkg.name} (and possibly each dependency
160+
listed below) to make the following decision:
161+
</p>
162+
<ul className="list-disc ml-8 mt-2">
163+
<li className="mb-2">
164+
<span className="font-semibold">
165+
Did a dependency introduce a new feature that {pkg.name}{' '}
166+
now uses? If so, you need to include the dependency in
167+
the release by bumping its version.
168+
</span>
169+
</li>
170+
<li>
171+
Once you've verified that changes to a dependency do not
172+
affect {pkg.name}, you may omit it from the release by
173+
pressing "Skip".
174+
</li>
175+
</ul>
176+
</>
177+
}
178+
/>
179+
)}
180+
{packageDependencyErrors[pkg.name].missingDependentNames.length >
181+
0 && (
182+
<DependencyErrorSection
183+
title="Missing Peer Dependents"
184+
items={packageDependencyErrors[pkg.name].missingDependentNames}
185+
setSelections={setSelections}
186+
errorSubject={`You've bumped ${pkg.name} by a major version, indicating that there are breaking changes. However, this package has peer dependents (that is, other packages that list this one as a peer dependency) that you should include in the release.`}
187+
errorDetails={
188+
<>
189+
<p className="mb-2">
190+
If a package has a peer dependency on other package, it
191+
means that a project which declares the first package as a
192+
dependency must also declare the second one in order for the
193+
first one to function correctly, and the version used for
194+
the second package must satisfy the requested version range.
195+
If this is not the case, a peer dependency warning will
196+
appear when dependencies are installed. (For instance, if{' '}
197+
<code className="font-mono">@metamask/foo-controller</code>{' '}
198+
has a peer dependency on{' '}
199+
<code className="font-mono">@metamask/bar-controller</code>{' '}
200+
^1.0.0, then if{' '}
201+
<code className="font-mono">@metamask/foo-controller</code>{' '}
202+
is present in a client's{' '}
203+
<code className="font-mono">package.json</code>,{' '}
204+
<code className="font-mono">@metamask/bar-controller</code>{' '}
205+
1.x must also be present.)
206+
</p>
207+
<p className="mb-2">
208+
If you release a new major version of {pkg.name}, and you
209+
upgrade it in a client, and you don't release any of the
210+
peer dependents listed below, you will receive a peer
211+
dependency warning, because all of the peer dependents
212+
expect the client to be using the previous version, and now
213+
that requirement is no longer satisfied.
214+
</p>
215+
<p>
216+
<span className="font-semibold">
217+
To fix this, you need to navigate to each of the
218+
dependents and include them in the release.
219+
</span>
220+
</p>
221+
</>
222+
}
223+
/>
224+
)}
175225
</div>
176226
)}
177227

0 commit comments

Comments
 (0)