Skip to content

Commit

Permalink
fix(ObjectStatus): fire onClick on SPACE or ENTER press (#5429)
Browse files Browse the repository at this point in the history
Fixes #5427
  • Loading branch information
Lukas742 committed Jan 19, 2024
1 parent e33c227 commit 5434770
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
6 changes: 6 additions & 0 deletions packages/main/src/components/ObjectStatus/ObjectStatus.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ describe('ObjectStatus', () => {
Content
</ObjectStatus>
);
cy.get('button').should('not.exist');
cy.findByText('Content').click();
cy.get('@clickSpy').should('not.be.called');
cy.findByRole('button').should('not.exist');
Expand All @@ -241,8 +242,13 @@ describe('ObjectStatus', () => {
Content
</ObjectStatus>
);
cy.get('button').should('be.visible');
cy.findByText('Content').click();
cy.get('@clickSpy').should('have.been.calledOnce');
cy.findByText('Content').realPress('Enter');
cy.get('@clickSpy').should('have.been.calledTwice');
cy.findByText('Content').realPress('Space');
cy.get('@clickSpy').should('have.been.calledThrice');
cy.findByText('Object Status Button').should('exist').and('not.be.visible');
});

Expand Down
3 changes: 3 additions & 0 deletions packages/main/src/components/ObjectStatus/ObjectStatus.jss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ const createInvertedIndicationStyles = (baseColor: string) => ({
});

const styles = {
normalizeCSS: {
all: 'unset'
},
objectStatus: {
fontFamily: ThemingParameters.sapFontFamily,
fontSize: ThemingParameters.sapFontSize,
Expand Down
32 changes: 22 additions & 10 deletions packages/main/src/components/ObjectStatus/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,18 @@ import styles from './ObjectStatus.jss.js';

export interface ObjectStatusPropTypes extends CommonProps {
/**
* Indicates if the ObjectStatus text and icon can be clicked/tapped by the user.
* Indicates if the ObjectStatus is rendered as inactive `div` or interactive `button` and therefore can be clicked/tapped by the user or not.
*
* **Note:** If this prop is set to `true`, you should also set the `children` or `icon` prop.
*
* **Note:** If you set this property to true, you have to also set the `children` or `icon` prop.
*
* @since 0.16.6
*/
active?: boolean;

/**
* Defines the icon in front of the `ObjectStatus` text.<br />
* Defines the icon in front of the `ObjectStatus` text.
*
* __Note:__ Although this slot accepts HTML Elements, it is strongly recommended that you only use `Icon` in order to preserve the intended design.
*/
icon?: ReactNode;
Expand All @@ -58,10 +60,11 @@ export interface ObjectStatusPropTypes extends CommonProps {
large?: boolean;

/**
* Defines the text of the `ObjectStatus`.<br />
* Defines the text of the `ObjectStatus`.
*
* __Note:__ Although this slot accepts HTML Elements, it is strongly recommended that you only use text in order to preserve the intended design.
*/
children?: string | number | ReactNode;
children?: ReactNode;

/**
* Defines the value state of the <code>ObjectStatus</code>. <br><br> Available options are: <ul> <li><code>None</code></li> <li><code>Error</code></li> <li><code>Warning</code></li> <li><code>Success</code></li> <li><code>Information</code></li> </ul>
Expand All @@ -71,7 +74,8 @@ export interface ObjectStatusPropTypes extends CommonProps {
state?: ValueState | keyof typeof ValueState | IndicationColor | keyof typeof IndicationColor;

/**
* Defines whether the default icon for each `ValueState` should be displayed.<br />
* Defines whether the default icon for each `ValueState` should be displayed.
*
* __Note:__ If the `icon` prop was set, `showDefaultIcon` has no effect.
*/
showDefaultIcon?: boolean;
Expand All @@ -93,9 +97,13 @@ export interface ObjectStatusPropTypes extends CommonProps {
/**
* Fires when the user clicks/taps on active text.
*
* __Note:__ This prop has no effect if `active` is not set to `true`.
*
* __Note:__ In order to support legacy code, `HTMLDivElement` is still supported even though the `click` event is never fired if the component isn't `active`.
*
* @since 0.16.6
*/
onClick?: MouseEventHandler<HTMLDivElement>;
onClick?: MouseEventHandler<HTMLButtonElement | HTMLDivElement>;
}

const getStateSpecifics = (state, showDefaultIcon, userIcon, stateAnnouncementText, i18nTexts) => {
Expand Down Expand Up @@ -151,7 +159,7 @@ const useStyles = createUseStyles(styles, { name: 'ObjectStatus' });
/**
* Status information that can be either text with a value state, or an icon.
*/
const ObjectStatus = forwardRef<HTMLDivElement, ObjectStatusPropTypes>((props, ref) => {
const ObjectStatus = forwardRef<HTMLDivElement | HTMLButtonElement, ObjectStatusPropTypes>((props, ref) => {
const {
state,
showDefaultIcon,
Expand Down Expand Up @@ -199,6 +207,7 @@ const ObjectStatus = forwardRef<HTMLDivElement, ObjectStatusPropTypes>((props, r
);

const objStatusClasses = clsx(
classes.normalizeCSS,
classes.objectStatus,
classes[`${state as string}`.toLowerCase()],
active && classes.active,
Expand All @@ -207,8 +216,11 @@ const ObjectStatus = forwardRef<HTMLDivElement, ObjectStatusPropTypes>((props, r
className
);

const TagName = active ? 'button' : 'div';

return (
<div
<TagName
// @ts-expect-error: both refs are allowed (attributes, etc. of HTMLButtonElement should only be used if `active` is `true`)
ref={ref}
className={objStatusClasses}
style={style}
Expand Down Expand Up @@ -241,7 +253,7 @@ const ObjectStatus = forwardRef<HTMLDivElement, ObjectStatusPropTypes>((props, r
{invisibleText}
</span>
)}
</div>
</TagName>
);
});

Expand Down

0 comments on commit 5434770

Please sign in to comment.