Repository navigation
feat(button)!: port to <pf-v6-button> - #3157
ArathyKumar wants to merge 32 commits into
Conversation
🦋 Changeset detectedLatest commit: f3a83f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Commitlint tests passed!More Info{
"valid": true,
"errors": [],
"warnings": [],
"input": "feat(button)!: port to `<pf-v6-button>`"
} |
Prevent Chromium Tab-wrap flakiness with a trailing focus sentinel, stop RovingTabindexController from stealing focus after Tab-out, pin timestamp fixtures to en-US, and add the missing major changeset for the v5→v6 button port. Assisted-By: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
✅ Deploy Preview for patternfly-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
<pf-v6-button>
…check Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # elements/pf-v5-clipboard-copy/pf-v5-clipboard-copy.ts # elements/pf-v5-tooltip/demo/block-triggers.html # elements/pf-v5-tooltip/demo/flip.html # elements/pf-v5-tooltip/demo/performance.html # elements/pf-v5-tooltip/demo/trigger.html # elements/pf-v5-tooltip/docs/pf-v5-tooltip.md
This comment has been minimized.
This comment has been minimized.
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai full review |
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
|
Hi @ArathyKumar, Hi @adamjohnson, I wonder if setting up sourcery-ai would help ? |
A FACE value set with setFormValue() participates in every form submission, unlike a native button whose value is only included when it is the submitter. Expose the value only while a type=submit button is actually submitting, so named type=button/reset controls and unclicked submit buttons cannot contribute spurious entries. Assisted-By: Claude Code <noreply@anthropic.com>
Components that render <pf-v6-button> still set --pf-v5-c-button-* custom properties, which the v6 button does not consume. Map padding, font size, link color, and the dropdown's visual disabled state onto --pf-v6-c-button-* surface and variant tokens. Assisted-By: Claude Code <noreply@anthropic.com>
Hiding an open listbox on focusout removes the focused option mid-Tab. When that happens before the browser finishes moving focus, the focus move is cancelled and focus lands back on the combobox. Defer hiding one animation frame when focus is moving outside the element, and give the select test a deterministic focus target after the fixture. Assisted-By: Claude Code <noreply@anthropic.com>
Add coverage for InternalsController form/shadowRoot getters and submit(submitter) fallback, and for RovingTabindexController not stealing focus when an index update occurs after focus left the widget. Assisted-By: Claude Code <noreply@anthropic.com>
The trailing tabindex=0 sentinel was injected into every test page to give Tab somewhere to go after a fixture, which changed focus behavior for all suites and forced weaker assertions in three specs. Remove it and restore the original assertions; Tab behavior is covered with per-fixture focus targets where needed. Assisted-By: Claude Code <noreply@anthropic.com>
Address code review findings: reflect the name attribute so programmatic name/value participate in FACE submission, classify the deprecated icon-position=right alias as icon-end, add a focus-visible outline, sync the count badge from the computed disabled state so fieldset disabling propagates, and cover these paths. Also correct the changeset migration notes for the v5 warning boolean and React-parity flag names. Assisted-By: Claude Code <noreply@anthropic.com>
…allback Address review findings: cancel the deferred listbox hide on disconnect and generation change, reuse the item list for the re-focus check so slotted options are recognized, add RovingTabindexController.focusItem for documented programmatic focus APIs and use it in chip groups, and rethrow non-TypeError failures from requestSubmit(submitter). Assisted-By: Claude Code <noreply@anthropic.com>
Add type=submit to migrated form controls that relied on the implicit v5 submit behavior, use variant=danger for the tabs demo delete button, and label the icon-only alert close button. These demos and the alert would otherwise render non-submitting controls or unlabeled buttons. Assisted-By: Claude Code <noreply@anthropic.com>
Mirror the pf-v6-spinner e2e pattern so the new element keeps demo snapshot and SSR coverage after the pf-v5-button e2e file was removed. Assisted-By: Claude Code <noreply@anthropic.com>
Stop overwriting tabindex on every update. Capture the first authored value, treat external changes (author or roving-tabindex controller) as the value to preserve, and restore it after link mode or disabled state temporarily removes the attribute. Add regression tests for authored, post-render, and disabled round-trip values. Assisted-By: Claude Code <noreply@anthropic.com>
The browser only performs implicit submission (Enter in a text field) when the form has a native submit button, which a FACE element is not. Render one visually hidden native submit button into each owning form via a WeakMap, shared by all submit buttons in that form, and remove it when the last one disconnects. SSR is gated with isServer; sr-only styles are inlined on the helper. Assisted-By: Claude Code <noreply@anthropic.com>
href renders an anchor on any variant, not only variant=link; narrow the README and changeset wording. Document the implicit-submission helper and the fact that name/value participates only on click. Assisted-By: Claude Code <noreply@anthropic.com>
Sync the helper's disabled state with the registered submit buttons, re-append the helper if the form's children are replaced, and on implicit submission report the default FACE button as SubmitEvent.submitter and expose its name/value for that submission (native default-button parity). Also detect first-render text from light-DOM child nodes so part=text and icon placement are correct before SlotController records populate, including SSR. Assisted-By: Claude Code <noreply@anthropic.com>
Components that rendered pf-v5-button now render pf-v6-button, so page-level --pf-v5-c-button-* overrides no longer apply to them. Assisted-By: Claude Code <noreply@anthropic.com>
The fixture class in the InternalsController spec omitted annotations on its `internals` property and `render` return type, which fails the CI build under --isolatedDeclarations. Assisted-By: Claude Code <noreply@anthropic.com>
ECMAScript static private identifiers are rejected in a class that uses the legacy `@customElement` decorator (TS18036). The implicit-submission handler is now an instance method invoked through a callback stored on the per-form registration. Also annotate the anchor type in the tabindex test so the elements project type-check passes. Assisted-By: Claude Code <noreply@anthropic.com>
ubuntu-latest now resolves to 26.04, which Playwright 1.57 does not support, so `npx playwright install` fails before any test runs. Assisted-By: Claude Code <noreply@anthropic.com>
Lit's server renderer instantiates the element without a DOM, so `this.childNodes` is undefined and rendering threw. Guard the child-node scan with `isServer`; slotted light DOM is not available to inspect on the server anyway. Assisted-By: Claude Code <noreply@anthropic.com>
The button's name/value participates on implicit (Enter) submissions as well as clicks, matching a native default submit button. The docs claimed otherwise. Assisted-By: Claude Code <noreply@anthropic.com>
|
There remain two spicy issues here
background: WICG/webcomponents#814 |
|
/agentic-review |
What I did
Ported the existing pf-v5-button Web Component to a new pf-v6-button Web Component that follows design and provides feature parity with the PatternFly v6 Button React component.
Changes
Testing Instructions
Notes to Reviewers
This PR ports the existing pf-v5-button Web Component to pf-v6-button and brings it in line with the PatternFly v6 React Button.
For more details about the requirements and expected behavior, please refer to the related issue #2984.