Skip to content

fix(button): block clicks and ENTER/SPACE on disabled anchor hosts (#DS-4364)#1630

Open
lskramarov wants to merge 7 commits into
mainfrom
fix/DS-4364
Open

fix(button): block clicks and ENTER/SPACE on disabled anchor hosts (#DS-4364)#1630
lskramarov wants to merge 7 commits into
mainfrom
fix/DS-4364

Conversation

@lskramarov

Copy link
Copy Markdown
Contributor

No description provided.

@lskramarov lskramarov self-assigned this Jun 11, 2026
@lskramarov lskramarov added the bug Something isn't working label Jun 11, 2026
@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 2e8264d):

https://koobiq-next--prs-1630-6vtwb8ip.web.app

(expires Sat, 20 Jun 2026 15:57:22 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c

@lskramarov lskramarov marked this pull request as draft June 11, 2026 06:55
@lskramarov lskramarov marked this pull request as ready for review June 11, 2026 12:54
@github-actions

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@github-actions

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@lskramarov

Copy link
Copy Markdown
Contributor Author

/approve-snapshots

@github-actions

Copy link
Copy Markdown

🔄 Updating snapshots.

@github-actions

Copy link
Copy Markdown

✅ Snapshots updated!

lskramarov and others added 5 commits June 11, 2026 19:08
…DS-4364)

- extend the disabled keydown guard to DOWN/LEFT/RIGHT arrows, not just ENTER/SPACE,
  since KbqDropdownTrigger also opens on those (Tab/Escape left untouched so focus can
  still leave a disabled but focusable <a kbq-button> host); fix the misleading comment
- dedupe the COMMENT_NODE literal by reusing getNodesWithoutComments at both levels
- tests: assert a disabled dropdown trigger stays closed on click/ENTER/SPACE/arrow
  (guards stopImmediatePropagation), the effect()-driven reveal while the content
  observer is disabled, aria-disabled removal on re-enable, parentTextElement wrapping
  textElement, and the dev-mode warning when the styler is used without KbqButton
# Conflicts:
#	packages/components/sidepanel/__screenshots__/03-light.png
@github-actions

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@lskramarov

Copy link
Copy Markdown
Contributor Author

/approve-snapshots

@github-actions

Copy link
Copy Markdown

🔄 Updating snapshots.

@github-actions

Copy link
Copy Markdown

✅ Snapshots updated!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the KbqButton implementation to properly block user interaction on disabled anchor hosts (e.g. <a kbq-button disabled>), while also refining button content layout to support explicit left/right icon slots and reliable text truncation.

Changes:

  • Block click and activation-related keydown events (ENTER/SPACE + relevant arrows) on disabled <a kbq-button> using capture-phase native listeners; add aria-disabled handling.
  • Introduce [kbqButtonLeftIcon] / [kbqButtonRightIcon] marker directives and update projection + styling logic to make icon placement order-independent.
  • Replace the previous overlay-based click interception and add a dedicated text container (.kbq-button-text) with truncation styles.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/public_api_guard/components/button.api.md Updates public API surface for new icon slot directives and refactors in KbqButton/KbqButtonCssStyler.
packages/docs-examples/components/button/button-content/button-content-example.ts Adds an example demonstrating order-independent icon slots.
packages/components/filter-bar/_filter-bar-theme.scss Removes styles targeting the removed .kbq-button-overlay.
packages/components/button/public-api.ts Exports the new button-icon entrypoint.
packages/components/button/button.scss Removes .kbq-button-overlay styling (overlay element removed).
packages/components/button/button.module.ts Registers and exports the new left/right icon marker directives.
packages/components/button/button.component.ts Implements capture-phase event blocking for disabled anchors; adds aria-disabled; refactors icon detection/styling and exposes title measurement elements.
packages/components/button/button.component.spec.ts Adds coverage for disabled-anchor click/keyboard blocking, dropdown trigger behavior, and new icon slot + truncation-related behavior.
packages/components/button/button.component.html Updates projection structure: left slot, text container, right slot; disables content observer when no icons.
packages/components/button/button-icon.ts Adds [kbqButtonLeftIcon] / [kbqButtonRightIcon] marker directives.
packages/components/button/_button-theme.scss Adjusts focus outline behavior to avoid showing keyboard focus on disabled (still-focus-retaining) anchors; removes overlay-related rules.
packages/components/button/_button-base.scss Adds .kbq-button-text with truncation and flex alignment; ensures wrapper/text can shrink for ellipsis.
packages/components-dev/button/template.html Adds dev showcase for icon slots and ellipsis behavior.
packages/components-dev/button/module.ts Adds KbqTitleModule to support the new ellipsis demo usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Directive({
selector: '[kbqButtonLeftIcon]'
})
export class KbqButtonLeftIcon {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

может лучше именовать как в form-field - prefix и suffix, что при добавлении rtl не было путаницы

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants