Skip to content

Fix: Use semantic button element instead of role="button" in ActiveFiltersBar (#27945)#10406

Open
KethniiImasha wants to merge 2 commits into
wso2:masterfrom
KethniiImasha:fix/prefer-tag-over-role-active-filters-bar
Open

Fix: Use semantic button element instead of role="button" in ActiveFiltersBar (#27945)#10406
KethniiImasha wants to merge 2 commits into
wso2:masterfrom
KethniiImasha:fix/prefer-tag-over-role-active-filters-bar

Conversation

@KethniiImasha

@KethniiImasha KethniiImasha commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Purpose

Replaced a <span role="button"> with a semantic <button> element in the filter remove button within active-filters-bar.tsx, as flagged by the React Doctor accessibility check (rule: react-doctor/prefer-tag-over-role).

Changes:

  • Replaced <span role="button" tabIndex={0} onKeyPress={...}> with <button type="button">, removing the now-redundant tabIndex and onKeyPress handler since native buttons handle keyboard interaction natively.
  • Added padding: 0 and font: inherit to .filter-remove-button in active-filters-bar.scss to preserve the existing visual appearance after switching from span to button.

Included a changeset (patch bump for @wso2is/admin.workflow-requests.v1).

Related Issues

Related PRs

  • N/A

Screenshots

Before (<span> tag)

Before

After (<button type="button">)

After

Technical Changes

  • Updated the tag wrapper inside the workflow active filters layout file from span to button.
  • Explicitly applied a type="button" attribute to suppress default HTML form submission side effects inside the filter container.
  • .filter-remove-button class already had background: transparent and border: none, so I added padding: 0 and font: inherit to neutralize the remaining default styling. This should keep the visual output identical to the "before" state.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

In ActiveFiltersBar, the per-filter remove control is changed from a span with role="button", tabIndex, and onKeyPress to a native button element with type="button". The .filter-remove-button SCSS rule adds padding: 0 and font: inherit to normalize button default styles. A patch changeset entry is added for @wso2is/admin.workflow-requests.v1.

Changes

ActiveFiltersBar semantic button fix

Layer / File(s) Summary
Span-to-button replacement, style reset, and changeset
features/admin.workflow-requests.v1/components/active-filters-bar.tsx, features/admin.workflow-requests.v1/components/active-filters-bar.scss, .changeset/slimy-tools-join.md
The filter-remove span (with role="button", tabIndex, onKeyPress) is replaced by a <button type="button">, keeping the same aria-label, onClick, and close icon. .filter-remove-button gains padding: 0 and font: inherit to neutralize native button defaults. A patch changeset entry records the change.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: replacing a role="button" span with a semantic button element in ActiveFiltersBar.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Changeset Required ✅ Passed Changeset file .changeset/slimy-tools-join.md exists and includes required packages (@wso2is/admin.workflow-requests.v1 and @wso2is/console) with patch version bumps.
Description check ✅ Passed The pull request description comprehensively addresses all required template sections with clear purpose, related issues, technical details, and supporting screenshots.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ create changeset

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.changeset/slimy-tools-join.md:
- Around line 1-5: The changeset file is missing the dependent package that
needs to be included per the coding guidelines. Since the change to
`@wso2is/admin.workflow-requests.v1` is in the features/ directory and
`@wso2is/console` depends on this package, you must add `@wso2is/console` with a
patch version bump to the changeset. Add a new line in the changeset file with
"`@wso2is/console`": patch alongside the existing
`@wso2is/admin.workflow-requests.v1` entry.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: e1e86a8e-6a91-48ab-aff6-93817f6b506f

📥 Commits

Reviewing files that changed from the base of the PR and between 1825338 and 4fb87cb.

📒 Files selected for processing (3)
  • .changeset/slimy-tools-join.md
  • features/admin.workflow-requests.v1/components/active-filters-bar.scss
  • features/admin.workflow-requests.v1/components/active-filters-bar.tsx

Comment thread .changeset/slimy-tools-join.md
@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.62%. Comparing base (1f86c1e) to head (6192d45).
⚠️ Report is 30 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10406   +/-   ##
=======================================
  Coverage   72.62%   72.62%           
=======================================
  Files         469      469           
  Lines       70633    70633           
  Branches      240      240           
=======================================
  Hits        51300    51300           
  Misses      19222    19222           
  Partials      111      111           
🚀 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.

@pavinduLakshan

Copy link
Copy Markdown
Member

Could you please add a screenshot of before and after UX of this change?

@KethniiImasha

Copy link
Copy Markdown
Contributor Author

Could you please add a screenshot of before and after UX of this change?

Hi @pavinduLakshan, here's the current ("before") behavior on the Workflow Requests page, showing the status: Approved filter tag with the existing span-based remove button:
before

The change replaces with . The .filter-remove-button class already had background: transparent and border: none, so I added padding: 0 and font: inherit to neutralize the remaining default styling, which should keep the visual output identical to the "before" state.
I set up a local IS instance to verify the "after" state from my dev console, but ran into an OAuth callback URL mismatch / connection issue getting the dev server running against it. Happy to keep debugging if an "after" screenshot is needed, or if the reasoning above is sufficient for review that works too, let me know!

@KethniiImasha

KethniiImasha commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Could you please add a screenshot of before and after UX of this change?

Hi @pavinduLakshan, I've added the screenshot of before and after UX changes to the PR description.

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.

[React Doctor] prefer-tag-over-role: Prefer the semantic <button> element over role="button" on a generic tag (1 occurrence)

2 participants