cps-dialog: focus ring on the dialog frame covers the dialog and blocks all clicks inside it

Đang mở Phù hợp với người mới
  • Có gì không ổn?
#948 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
72/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
angular, css, playwright, scss, typescript
Lĩnh vực
frontend

Hướng nghiên cứu

Bắt đầu từ mixin focus-ring trong projects/cps-ui-kit/styles/_mixins.scss và quy tắc :focus-visible trong cps-dialog.component.scss quanh dòng 107. Để tái hiện, mở ví dụ dialog có thể kéo, nhấp vào phần tiêu đề, nhấn ArrowDown và kiểm tra xem elementFromPoint trả về dialog thay vì nút. Hoàn thành khi các pseudo-element của vòng focus không còn chặn click, vòng focus trông không đổi, và một spec Playwright đặt cạnh các test focus-ring nhấp thành công vào «Close dialog».

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

bug

Summary

When a cps-dialog has keyboard focus on its frame, the focus ring drawn around the dialog covers the whole dialog and takes every pointer event. Buttons, inputs and links inside the dialog stop responding to clicks until the focus moves elsewhere.

The state is easy to reach: click on the dialog's header, then press a key. The click focuses the dialog frame (div.cps-dialog[tabindex="-1"]), and the key press makes the browser treat that focus as keyboard focus (:focus-visible matches), which turns the ring on.

Environment

  • cps-ui-kit 21.33.0. The source of 22.3.0 has the same CSS for this ring, but I did not run that version.
  • Reproduced in Chromium and WebKit (automated browsers).
  • Reproduced on the kit's own documentation site, so it does not depend on any application code. I do not know which kit version the site is currently built with.

Steps to reproduce

  1. Open https://absaoss.github.io/cps-shared-ui/dialog/examples and click the Draggable dialog button.
  2. In the dialog, click Disable closing. This stops Esc from closing the dialog, which would otherwise hide the problem. (Skipping this step and pressing ArrowDown in step 4 gives the same result.)
  3. Click on the dialog's header, in a bare part of it.
  4. Press Esc (or ArrowDown).
  5. Click anything inside the dialog, for example the Close dialog button.

Expected

The dialog shows its focus ring, and everything inside it still responds to clicks.

Actual

The dialog shows its focus ring, but clicks on everything inside it do nothing. The cursor behaves as if the dialog were one inert block.

Observed in the browser after step 4:

  • document.activeElement is div.cps-dialog, and .matches(':focus-visible') is true.
  • document.elementFromPoint(...) at the centre of the Close dialog button returns div.cps-dialog, not the button.
  • getComputedStyle(dialog, '::before') is content: "", inset: -1px, border: 1px solid, pointer-events: auto. ::after is the same with inset: -2px.
  • Playwright's click on the button fails with: <div role="dialog" tabindex="-1" data-testid="cps-dialog" ... class="cps-dialog ..."> intercepts pointer events.

Cause

The dialog frame's focus ring comes from the focus-ring mixin:

  • projects/cps-ui-kit/src/lib/services/cps-dialog/internal/components/cps-dialog/cps-dialog.component.scss, around line 107: &:focus-visible { @include focus-ring(0.0625rem, 0.125rem, inherit); }
  • projects/cps-ui-kit/styles/_mixins.scss, @mixin focus-ring: it sets position: relative on the element and draws the ring with ::before and ::after, both position: absolute with a negative inset and a border.

On a small control that is harmless. On the dialog frame the two pseudo-elements are as large as the whole dialog. They are positioned, so they paint above the dialog's non-positioned content, and because pointer-events is left at its default they become the hit-test target for every point inside the dialog.

Esc is only one way in. A dialog whose closeOnEscape is false (or disableClose is set) stays open after Esc, so it is the clearest repro, but any key press after the header click does it: ArrowDown works on the docs site in both browsers. In WebKit, Shift alone does not turn the ring on, but ArrowDown and Esc do.

Suggested fix

The ring is decorative, so it should never be a pointer target. The smallest change is in the mixin, which also protects every other component that uses it:

&::before,
&::after {
  content: '';
  position: absolute;
  pointer-events: none; // the ring is decorative and must never take clicks
  border-radius: $radius;
}

If changing the mixin is too broad, put the same pointer-events: none on the ring's pseudo-elements for the dialog frame only.

The ring must stay visible; only its pointer behaviour changes. Other uses of the mixin on large containers (for example the resizable handles in the same file, around lines 195 and 285) may be worth checking for the same effect. I did not test those.

Acceptance criteria

  • After clicking the dialog header and pressing Esc (with closeOnEscape: false) or ArrowDown, buttons and inputs inside the dialog can be clicked.
  • The focus ring is still drawn exactly as before.
  • A Playwright test covers it, next to the existing focus-ring specs under playwright/cps-ui-kit/components/: open the draggable dialog example, click the header, press ArrowDown, then click Close dialog and expect the dialog to close.

Workaround for applications

.cps-dialog:focus-visible::before,
.cps-dialog:focus-visible::after {
  pointer-events: none;
}
Ngôn ngữ chính
TypeScript
Star
9
Fork
8
Merge trung bình
1 ngày 6 giờ
Pull request đã merge (30 ngày)
25

Chuẩn bị môi trường

Dự án này không cung cấp dev container, Dockerfile hay hướng dẫn đóng góp, nên bạn cần tự thiết lập môi trường: hãy bắt đầu từ README và xem hướng dẫn đóng góp lần đầu của chúng tôi để biết các bước chung.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của AbsaOSS/cps-shared-ui

Tất cả issue của AbsaOSS/cps-shared-ui

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.