-
Notifications
You must be signed in to change notification settings - Fork 393
draft - Button next with PFE #12638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
kmcfaul
wants to merge
1
commit into
patternfly:main
Choose a base branch
from
kmcfaul:felt-button-exploration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+425
−1
Draft
draft - Button next with PFE #12638
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
259 changes: 259 additions & 0 deletions
259
packages/react-core/src/components/ButtonNext/FeltButton.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,259 @@ | ||
| import { forwardRef, type ReactNode, type Ref } from 'react'; | ||
| import type { OUIAProps } from '../../helpers/OUIA/ouia'; | ||
| import type { PfV5Button as PfV5ButtonElement } from '@patternfly/elements/pf-v5-button/pf-v5-button.js'; | ||
| import { V5Button } from '@patternfly/elements/react/pf-v5-button/pf-v5-button.js'; | ||
| import type { BadgeCountObject } from '../Button'; | ||
|
|
||
| type PfButtonVariant = | ||
| | 'primary' | ||
| | 'secondary' | ||
| | 'tertiary' | ||
| | 'danger' | ||
| | 'warning' | ||
| | 'link' | ||
| | 'plain' | ||
| | 'control' | ||
| | 'stateful'; | ||
|
|
||
| export interface FeltButtonProps extends OUIAProps { | ||
| children?: ReactNode; | ||
| className?: string; | ||
| component?: React.ElementType<any> | React.ComponentType<any>; | ||
| isClicked?: boolean; | ||
| isBlock?: boolean; | ||
| isDisabled?: boolean; | ||
| isAriaDisabled?: boolean; | ||
| isLoading?: boolean; | ||
| spinnerAriaValueText?: string; | ||
| spinnerAriaLabel?: string; | ||
| spinnerAriaLabelledBy?: string; | ||
| inoperableEvents?: string[]; | ||
| isInline?: boolean; | ||
| isFavorite?: boolean; | ||
| isFavorited?: boolean; | ||
| size?: 'default' | 'sm' | 'lg'; | ||
| type?: 'button' | 'submit' | 'reset'; | ||
| variant?: PfButtonVariant; | ||
| state?: 'read' | 'unread' | 'attention'; | ||
| hasNoPadding?: boolean; | ||
| iconPosition?: 'start' | 'end' | 'left' | 'right'; | ||
| 'aria-label'?: string; | ||
| icon?: ReactNode | null; | ||
| tabIndex?: number; | ||
| isDanger?: boolean; | ||
| isExpanded?: boolean; | ||
| isSettings?: boolean; | ||
| isHamburger?: boolean; | ||
| hamburgerVariant?: 'expand' | 'collapse'; | ||
| isCircle?: boolean; | ||
| isDocked?: boolean; | ||
| isTextExpanded?: boolean; | ||
| countOptions?: BadgeCountObject; | ||
| ouiaId?: number | string; | ||
| ouiaSafe?: boolean; | ||
| onClick?: React.MouseEventHandler; | ||
| href?: string; | ||
| target?: string; | ||
| id?: string; | ||
| style?: React.CSSProperties; | ||
| title?: string; | ||
| name?: string; | ||
| value?: string; | ||
| role?: string; | ||
| [key: `data-${string}`]: string | undefined; | ||
| [key: `aria-${string}`]: string | undefined; | ||
| } | ||
|
|
||
| const unsupportedProps: Record<string, string> = { | ||
| component: | ||
| 'The "component" prop is not supported. pf-v5-button renders as a <button> (or <a> for link+href). Use variant="link" with href for anchor behavior.', | ||
| isAriaDisabled: 'The "isAriaDisabled" prop is not supported. pf-v5-button only supports native disabled.', | ||
| isClicked: 'The "isClicked" prop is not supported by pf-v5-button.', | ||
| isFavorite: 'The "isFavorite" prop is not supported by pf-v5-button.', | ||
| isFavorited: 'The "isFavorited" prop is not supported by pf-v5-button.', | ||
| isSettings: 'The "isSettings" prop is not supported by pf-v5-button.', | ||
| isHamburger: 'The "isHamburger" prop is not supported by pf-v5-button.', | ||
| hamburgerVariant: 'The "hamburgerVariant" prop is not supported by pf-v5-button.', | ||
| isCircle: 'The "isCircle" prop is not supported by pf-v5-button.', | ||
| isDocked: 'The "isDocked" prop is not supported by pf-v5-button.', | ||
| isTextExpanded: 'The "isTextExpanded" prop is not supported by pf-v5-button.', | ||
| countOptions: 'The "countOptions" prop is not supported by pf-v5-button.', | ||
| hasNoPadding: 'The "hasNoPadding" prop is not supported by pf-v5-button.', | ||
| inoperableEvents: 'The "inoperableEvents" prop is not supported. pf-v5-button does not support aria-disabled.', | ||
| spinnerAriaValueText: 'The "spinnerAriaValueText" prop is not supported. Use the "loading-label" attribute instead.', | ||
| spinnerAriaLabelledBy: 'The "spinnerAriaLabelledBy" prop is not supported by pf-v5-button.', | ||
| ouiaId: 'The "ouiaId" prop is not supported by pf-v5-button.', | ||
| ouiaSafe: 'The "ouiaSafe" prop is not supported by pf-v5-button.', | ||
| state: 'The "state" prop (stateful variant) is not supported by pf-v5-button.', | ||
| isExpanded: 'The "isExpanded" prop is not supported by pf-v5-button.' | ||
| }; | ||
|
|
||
| const warnedProps = new Set<string>(); | ||
|
|
||
| function warnUnsupported(propName: string) { | ||
| if (warnedProps.has(propName)) { | ||
| return; | ||
| } | ||
| warnedProps.add(propName); | ||
| const message = unsupportedProps[propName]; | ||
| if (message) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(`FeltButton: ${message}`); | ||
| } | ||
| } | ||
|
|
||
| function mapSizeToPfe(size?: 'default' | 'sm' | 'lg'): 'small' | 'large' | undefined { | ||
| switch (size) { | ||
| case 'sm': | ||
| return 'small'; | ||
| case 'lg': | ||
| return 'large'; | ||
| default: | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| function resolveVariant( | ||
| variant: PfButtonVariant = 'primary', | ||
| isDanger?: boolean | ||
| ): { | ||
| pfeVariant: 'primary' | 'secondary' | 'tertiary' | 'control' | 'link'; | ||
| danger: boolean; | ||
| warning: boolean; | ||
| plain: boolean; | ||
| } { | ||
| switch (variant) { | ||
| case 'danger': | ||
| return { pfeVariant: 'primary', danger: true, warning: false, plain: false }; | ||
| case 'warning': | ||
| return { pfeVariant: 'primary', danger: false, warning: true, plain: false }; | ||
| case 'plain': | ||
| return { pfeVariant: 'primary', danger: false, warning: false, plain: true }; | ||
| case 'stateful': | ||
| return { pfeVariant: 'primary', danger: false, warning: false, plain: false }; | ||
| default: | ||
| return { | ||
| pfeVariant: variant, | ||
| danger: !!(isDanger && (variant === 'secondary' || variant === 'link')), | ||
| warning: false, | ||
| plain: false | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| const FeltButtonBase = ( | ||
| { | ||
| children = null, | ||
| className, | ||
| component, | ||
| isClicked, | ||
| isBlock = false, | ||
| isDisabled = false, | ||
| isAriaDisabled, | ||
| isLoading, | ||
| spinnerAriaValueText, | ||
| spinnerAriaLabel, | ||
| spinnerAriaLabelledBy, | ||
| inoperableEvents, | ||
| isInline = false, | ||
| isFavorite, | ||
| isFavorited, | ||
| size = 'default', | ||
| type = 'button', | ||
| variant = 'primary', | ||
| state, | ||
| hasNoPadding, | ||
| iconPosition = 'start', | ||
| 'aria-label': ariaLabel, | ||
| icon = null, | ||
| tabIndex, | ||
| isDanger, | ||
| isExpanded, | ||
| isSettings, | ||
| isHamburger, | ||
| hamburgerVariant, | ||
| isCircle, | ||
| isDocked, | ||
| isTextExpanded, | ||
| countOptions, | ||
| ouiaId, | ||
| ouiaSafe, | ||
| onClick, | ||
| href, | ||
| target, | ||
| name, | ||
| value, | ||
| ...rest | ||
| }: FeltButtonProps, | ||
| ref: Ref<PfV5ButtonElement> // replaces hidden prop innerRef with native forwardRef | ||
| ) => { | ||
| // Warn for unsupported props (once per prop) | ||
| const unsupportedValues: Record<string, unknown> = { | ||
| component, | ||
| isAriaDisabled, | ||
| isClicked, | ||
| isFavorite, | ||
| isFavorited, | ||
| isSettings, | ||
| isHamburger, | ||
| hamburgerVariant, | ||
| isCircle, | ||
| isDocked, | ||
| isTextExpanded, | ||
| countOptions, | ||
| hasNoPadding, | ||
| inoperableEvents, | ||
| spinnerAriaValueText, | ||
| spinnerAriaLabelledBy, | ||
| ouiaId, | ||
| ouiaSafe, | ||
| state, | ||
| isExpanded | ||
| }; | ||
| for (const prop of Object.keys(unsupportedProps)) { | ||
| if (unsupportedValues[prop] !== undefined && unsupportedValues[prop] !== false) { | ||
| warnUnsupported(prop); | ||
| } | ||
| } | ||
| if (variant === 'stateful') { | ||
| warnUnsupported('state'); | ||
| } | ||
|
|
||
| const { pfeVariant, danger, warning, plain } = resolveVariant(variant, isDanger); | ||
| const isIconAtEnd = iconPosition === 'end' || iconPosition === 'right'; | ||
|
|
||
| return ( | ||
| <V5Button | ||
| ref={ref} | ||
| className={className} | ||
| variant={pfeVariant} | ||
| danger={danger} | ||
| warning={warning} | ||
| plain={plain} | ||
| disabled={isDisabled} | ||
| loading={isLoading || false} | ||
| inline={isInline} | ||
| block={isBlock} | ||
| size={mapSizeToPfe(size)} | ||
| type={type} | ||
| label={ariaLabel || undefined} | ||
| name={name} | ||
| value={value} | ||
| href={pfeVariant === 'link' ? href : undefined} | ||
| target={pfeVariant === 'link' && href ? target : undefined} | ||
| loading-label={spinnerAriaLabel} | ||
| tabIndex={tabIndex} | ||
| onClick={onClick} | ||
| {...rest} | ||
| > | ||
| {/* icon position doesn't work - slot is static inside web component. would need a start and end icon slot to support. */} | ||
| {icon && !isIconAtEnd && <span slot="icon">{icon}</span>} | ||
| {children} | ||
| {icon && isIconAtEnd && <span slot="icon">{icon}</span>} | ||
| </V5Button> | ||
| ); | ||
| }; | ||
|
|
||
| export const FeltButton = forwardRef<PfV5ButtonElement, FeltButtonProps>(FeltButtonBase); | ||
|
|
||
| FeltButton.displayName = 'FeltButton'; | ||
20 changes: 20 additions & 0 deletions
20
packages/react-core/src/components/ButtonNext/examples/FeltButton.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| --- | ||
| id: Button | ||
| section: components | ||
| source: react-next | ||
| propComponents: ['FeltButton', 'BadgeCountObject'] | ||
| ouia: true | ||
| --- | ||
|
|
||
| import RhMicronsCloseIcon from '@patternfly/react-icons/dist/esm/icons/rh-microns-close-icon'; | ||
| import RhUiExternalLinkFillIcon from '@patternfly/react-icons/dist/esm/icons/rh-ui-external-link-fill-icon'; | ||
| import RhUiAddCircleFillIcon from '@patternfly/react-icons/dist/esm/icons/rh-ui-add-circle-fill-icon'; | ||
| import RhUiCopyFillIcon from '@patternfly/react-icons/dist/esm/icons/rh-ui-copy-fill-icon'; | ||
|
|
||
| ## Examples | ||
|
|
||
| ### Variant examples | ||
|
|
||
| ```ts file="./FeltButtonVariations.tsx" | ||
|
|
||
| ``` |
42 changes: 42 additions & 0 deletions
42
packages/react-core/src/components/ButtonNext/examples/FeltButtonVariations.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { FeltButton } from '@patternfly/react-core/components/ButtonNext'; | ||
| import { Flex } from '@patternfly/react-core'; | ||
| import RhMicronsCloseIcon from '@patternfly/react-icons/dist/esm/icons/rh-microns-close-icon'; | ||
| import RhUiExternalLinkFillIcon from '@patternfly/react-icons/dist/esm/icons/rh-ui-external-link-fill-icon'; | ||
| import RhUiAddCircleFillIcon from '@patternfly/react-icons/dist/esm/icons/rh-ui-add-circle-fill-icon'; | ||
| import RhUiCopyFillIcon from '@patternfly/react-icons/dist/esm/icons/rh-ui-copy-fill-icon'; | ||
|
|
||
| export const FeltButtonVariations: React.FunctionComponent = () => ( | ||
| <> | ||
| <Flex columnGap={{ default: 'columnGapSm' }}> | ||
| <FeltButton variant="primary">Primary</FeltButton> | ||
| <FeltButton variant="secondary">Secondary</FeltButton> | ||
| <FeltButton variant="secondary" isDanger> | ||
| Danger Secondary | ||
| </FeltButton> | ||
| <FeltButton variant="tertiary">Tertiary</FeltButton> | ||
| <FeltButton variant="danger">Danger</FeltButton> | ||
| <FeltButton variant="warning">Warning</FeltButton> | ||
| </Flex> | ||
| <br /> | ||
| <Flex columnGap={{ default: 'columnGapSm' }}> | ||
| <FeltButton variant="link" icon={<RhUiAddCircleFillIcon />}> | ||
| Link | ||
| </FeltButton> | ||
| <FeltButton variant="link" icon={<RhUiExternalLinkFillIcon />} iconPosition="end"> | ||
| Link | ||
| </FeltButton> | ||
| <FeltButton variant="link" isInline> | ||
| Inline link | ||
| </FeltButton> | ||
| <FeltButton variant="link" isDanger> | ||
| Danger link | ||
| </FeltButton> | ||
| <FeltButton variant="plain" aria-label="Action" icon={<RhMicronsCloseIcon />} /> | ||
| </Flex> | ||
| <br /> | ||
| <Flex columnGap={{ default: 'columnGapSm' }}> | ||
| <FeltButton variant="control">Control</FeltButton> | ||
| <FeltButton variant="control" aria-label="Copy" icon={<RhUiCopyFillIcon />} /> | ||
| </Flex> | ||
| </> | ||
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './FeltButton'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🌐 Web query:
For@patternfly/elements5.x, determine whether a slottedslot="icon"React child can render at the end ofpf-v5-buttoncontent, or whethericon-positiononly applies to the element's generated icon property.💡 Result:
In
@patternfly/elements5.x, the pf-v5-button component handles icons primarily through the icon property/attribute, which provides a shorthand for rendering icons within the button [1][2]. While the component supports an icon slot for custom icon content, the icon-position attribute (or property) is specifically designed to control the positioning of the icon rendered via the icon property [1][3]. Regarding your specific questions: 1. Slotted icon content: You can use the icon slot to provide custom content (such as an or ) [1][2]. However, the icon-position property is intended to manage the layout of the icon set via the component's icon property, not necessarily to reorder arbitrary slotted elements [1][2]. If you place an element with slot="icon" inside the button, it will render, but the built-in icon-position logic may not automatically manipulate the position of that manual slot relative to your text content in the same way it manages the shorthand icon property [1][2]. 2. Scope of icon-position: The icon-position attribute applies to the icon specified by the component's icon property [1][3]. It determines whether that icon appears at the start (left) or end (right) of the button text [3]. For complex layouts or custom icon placement, it is often more predictable to manually order your elements within the button's default slot rather than relying on the component's shorthand icon management features [1].Citations:
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 10143
🤖 get_repo_knowledge executed:
get_repo_knowledge patternfly/patternfly-react /tmp/coderabbit-repo-knowledge/patternfly-patternfly-react-228bfecaLength of output: 657
🌐 Web query:
@patternfly/elements 5.0.0 pf-v5-button source icon slot icon-position💡 Result:
In
@patternfly/elementsversion 5.0.0, the button component is named pf-v5-button [1][2]. The icon is managed via an icon attribute and an icon-position attribute, which determines where the icon appears in relation to the button text [3]. Key details for pf-v5-button icon management: Icon Attribute: You can define the icon using the icon attribute, which accepts the icon name (e.g., icon="arrow") [3]. This acts as a shorthand for placing an icon within the component [3]. Icon Position: The icon-position attribute is used to set the placement of the icon [3]. Supported values typically include "right" (defaulting to the end of the button content) [3]. Note on Styling: While the PatternFly core CSS uses logical modifiers such as.pf-m-start and.pf-m-end for positioning icons (where "start" and "end" are preferred over "left" and "right" for RTL/LTR support) [4][5][6], the@patternfly/elementspf-v5-button element abstracts this via its attributes [3]. Example usage: Label If you are migrating from older versions, note that all elements in version 5.0.0 now carry the pf-v5- prefix [2], and you should import them individually from@patternfly/elements/pf-v5-button/pf-v5-button.js[3][2].Citations:
🌐 Web query:
site:github.com/patternfly/patternfly-elements pf-v5-button icon-position slot="icon"💡 Result:
In the PatternFly Elements library, the
pf-v5-buttoncomponent (part of the v5 element series) supports icons through property-based configuration rather than exclusively via a slot named "icon" [1][2]. While your query mentionsicon-position slot="icon", standard usage forpf-v5-buttoninvolves using theiconattribute to specify the icon name [2]. For example: Search The component handles positioning and spacing automatically based on the button's content [3]. If you need to include custom elements or specific icon configurations that the simple attribute does not support, you may use standard slotting techniques if supported by the component's internal Shadow DOM template, though the primary API for icons is theiconattribute [2]. Please note that PatternFly Elements recently introduced a versioning scheme where components are prefixed withpf-v5-[4][5]. Ensure you are using the correct version imports (e.g.,import '@patternfly/elements/pf-v5-button/pf-v5-button.js') as documented in the library [4].Citations:
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 7285
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 275
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 5569
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 2267
Implement or reject trailing icon placement.
iconPosition="end"only changes the light-DOM child order.FeltButtondoes not passicon-positiontoV5Button, andpf-v5-buttonrenders its fixediconslot before itstextslot. The trailing-icon example therefore renders the icon before the label. Support trailing placement or warn for"end"and"right".🤖 Prompt for AI Agents