Skip to content

Commit

Permalink
fix: a11y issues in the overlay component (HMR) (#49782)
Browse files Browse the repository at this point in the history
This is a follow-up PR for #49460.

I've fixed some a11y issues at #49460, but I've noticed that Next has other overlay implementations.
So I've applied that into another implementation, which seems to be for HMR overlay.

There is another overlay implementation for Turbopack, so I'll fix it later.

- https://github.com/vercel/next.js/tree/canary/packages/react-dev-overlay/src/internal
- https://github.com/vercel/next.js/tree/canary/packages/next/src/client/components/react-dev-overlay/internal
- https://github.com/vercel/next.js/tree/canary/packages/next-swc/crates/next-core/js/src/overlay/internal

I couldn't get the way to run an application with `--turbo` in the monorepo by `next-with-deps`. I tried `pnpm next-with-deps --turbo ./examples/app-dir-mdx`, but I've got the `Error: Cannot find module 'next/dist/server/node-polyfill-fetch'` error  and couldn't fix it. I tried building `next-swc` by `pnpm build-native`, but couldn't fix the error.
How can I run a local application with `--turbo`?

I expect this is a temporary fix because it would be nice if I could remove duplicated components by removing duplicated components and importing from one place.
I feel `packages/react-dev-overlay` is the right place for this because it's a separate package. But `packages/react-dev-overlay` imports some modules from `next/dist/client/components/react-dev-overlay/`, which is an opposite dependency.

e.g. https://github.com/vercel/next.js/blob/afddb6ebdade616cdd7780273be4cd28d4509890/packages/react-dev-overlay/src/client.ts#L3

I've also found `@next/react-dev-ovrelay` is imported from `package/next` as a combiled package like `next/dist/compiled/@next/react-dev-overlay/dist/client`

- https://github.com/vercel/next.js/blob/afddb6ebdade616cdd7780273be4cd28d4509890/packages/next/src/client/dev/error-overlay/hot-dev-client.ts#L31-L37
- https://github.com/vercel/next.js/blob/afddb6ebdade616cdd7780273be4cd28d4509890/packages/next/src/server/dev/next-dev-server.ts#L61-L66

What do you think? I'll work on this if it makes sense.
  • Loading branch information
koba04 committed Jun 2, 2023
1 parent 0f7da4c commit a7afb53
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ const LeftRightDialogHeader: React.FC<LeftRightDialogHeaderProps> =
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<title>previous</title>
<path
d="M6.99996 1.16666L1.16663 6.99999L6.99996 12.8333M12.8333 6.99999H1.99996H12.8333Z"
stroke="currentColor"
Expand All @@ -135,6 +136,7 @@ const LeftRightDialogHeader: React.FC<LeftRightDialogHeaderProps> =
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<title>next</title>
<path
d="M6.99996 1.16666L12.8333 6.99999L6.99996 12.8333M1.16663 6.99999H12H1.16663Z"
stroke="currentColor"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,12 @@ export const styles = css`
color: var(--color-ansi-red);
}
.nextjs-container-errors-body > h5:not(:first-child) {
.nextjs-container-errors-body > h2:not(:first-child) {
margin-top: calc(var(--size-gap-double) + var(--size-gap));
}
.nextjs-container-errors-body > h5 {
.nextjs-container-errors-body > h2 {
margin-bottom: var(--size-gap);
font-size: var(--size-font-big);
}
.nextjs-toast-errors-parent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ export const CallStackFrame: React.FC<{

return (
<div data-nextjs-call-stack-frame>
<h6 data-nextjs-frame-expanded={Boolean(frame.expanded)}>
<h3 data-nextjs-frame-expanded={Boolean(frame.expanded)}>
{f.methodName}
</h6>
</h3>
<div
data-has-source={hasSource ? 'true' : undefined}
tabIndex={hasSource ? 10 : undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function ComponentStackFrameRow({

return (
<div data-nextjs-component-stack-frame>
<h6>{component}</h6>
<h3>{component}</h3>
{file ? (
<div
tabIndex={10} // match CallStackFrame
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const RuntimeError: React.FC<RuntimeErrorProps> = function RuntimeError({
<React.Fragment>
{firstFrame ? (
<React.Fragment>
<h5>Source</h5>
<h2>Source</h2>
{leadingFrames.map((frame, index) => (
<CallStackFrame
key={`leading-frame-${index}-${all}`}
Expand All @@ -88,7 +88,7 @@ const RuntimeError: React.FC<RuntimeErrorProps> = function RuntimeError({

{error.componentStackFrames ? (
<>
<h5>Component Stack</h5>
<h2>Component Stack</h2>
{error.componentStackFrames.map((componentStackFrame, index) => (
<ComponentStackFrameRow
key={index}
Expand All @@ -100,7 +100,7 @@ const RuntimeError: React.FC<RuntimeErrorProps> = function RuntimeError({

{stackFramesGroupedByFramework.length ? (
<React.Fragment>
<h5>Call Stack</h5>
<h2>Call Stack</h2>
<GroupedStackFrames
groupedStackFrames={stackFramesGroupedByFramework}
all={all}
Expand Down Expand Up @@ -138,14 +138,15 @@ export const styles = css`
margin-bottom: var(--size-gap-double);
}
[data-nextjs-call-stack-frame] > h6,
[data-nextjs-component-stack-frame] > h6 {
[data-nextjs-call-stack-frame] > h3,
[data-nextjs-component-stack-frame] > h3 {
margin-top: 0;
margin-bottom: var(--size-gap);
font-family: var(--font-stack-monospace);
font-size: var(--size-font);
color: #222;
}
[data-nextjs-call-stack-frame] > h6[data-nextjs-frame-expanded='false'] {
[data-nextjs-call-stack-frame] > h3[data-nextjs-frame-expanded='false'] {
color: #666;
}
[data-nextjs-call-stack-frame] > div,
Expand Down Expand Up @@ -201,7 +202,7 @@ export const styles = css`
display: none;
}
[data-nextjs-collapsed-call-stack-details] h6 {
[data-nextjs-collapsed-call-stack-details] h3 {
color: #666;
}
[data-nextjs-collapsed-call-stack-details] [data-nextjs-call-stack-frame] {
Expand Down

0 comments on commit a7afb53

Please sign in to comment.