Skip to content
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

Rename NavigationLink component to NavigationAction #1825

Merged
merged 2 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,33 @@
import classNames from 'classnames';
import React, { ElementType, forwardRef } from 'react';
import { useStyleProps } from '../../hooks';
import { PolymorphicRef, SpiritNavigationLinkProps } from '../../types';
import { useNavigationLinkProps } from './useNavigationLinkProps';
import { useNavigationLinkStyleProps } from './useNavigationLinkStyleProps';
import { PolymorphicRef, SpiritNavigationActionProps } from '../../types';
import { useNavigationActionProps } from './useNavigationActionProps';
import { useNavigationActionStyleProps } from './useNavigationActionStyleProps';

const defaultProps: Partial<SpiritNavigationLinkProps> = {
const defaultProps: Partial<SpiritNavigationActionProps> = {
elementType: 'a',
};

/* We need an exception for components exported with forwardRef */
/* eslint no-underscore-dangle: ['error', { allow: ['_NavigationLink'] }] */
const _NavigationLink = <E extends ElementType = 'a'>(
props: SpiritNavigationLinkProps<E>,
/* eslint no-underscore-dangle: ['error', { allow: ['_NavigationAction'] }] */
const _NavigationAction = <E extends ElementType = 'a'>(
props: SpiritNavigationActionProps<E>,
ref: PolymorphicRef<E>,
): JSX.Element => {
const propsWithDefaults = { ...defaultProps, ...props };
const { elementType = defaultProps.elementType as ElementType, children, ...restProps } = propsWithDefaults;
const ElementTag = propsWithDefaults.isDisabled ? 'span' : elementType;

const { navigationLinkProps } = useNavigationLinkProps(propsWithDefaults);
const { classProps, props: modifiedProps } = useNavigationLinkStyleProps(restProps);
const { navigationActionProps } = useNavigationActionProps(propsWithDefaults);
const { classProps, props: modifiedProps } = useNavigationActionStyleProps(restProps);
const { styleProps, props: otherProps } = useStyleProps(modifiedProps);

return (
<ElementTag
{...otherProps}
{...styleProps}
{...navigationLinkProps}
{...navigationActionProps}
className={classNames(classProps, styleProps.className)}
ref={ref}
>
Expand All @@ -38,6 +38,6 @@ const _NavigationLink = <E extends ElementType = 'a'>(
);
};

const NavigationLink = forwardRef<HTMLElement, SpiritNavigationLinkProps<ElementType>>(_NavigationLink);
const NavigationAction = forwardRef<HTMLElement, SpiritNavigationActionProps<ElementType>>(_NavigationAction);

export default NavigationLink;
export default NavigationAction;
60 changes: 30 additions & 30 deletions packages/web-react/src/components/Navigation/README.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# Navigation

The `Navigation` component is a container for the navigation links of the application.
The `Navigation` component is a container for the navigation actions of the application.

It consists of a these parts:

- [Navigation](#navigation)
- [NavigationItem](#navigation-item)
- [NavigationLink](#navigation-link)
- [NavigationItem](#navigation-item)
- [NavigationAction](#navigation-action)

## Navigation

Expand All @@ -18,10 +18,10 @@ import { Navigation } from '@lmc-eu/spirit-web-react';
<Navigation aria-label="Main Navigation">{/* Navigation items go here */}</Navigation>;
```

It centres its children vertically, and if the children do not include `NavigationLink` components,
It centres its children vertically, and if the children do not include `NavigationAction` components,
it will apply a gap between them.

ℹ️ Don't forget to add the `aria-label` attribute to the `Navigation` component for correct accessible state.
ℹ️ Don't forget to add the `aria-label` attribute to the `Navigation` component for correct accessible title.

### API

Expand All @@ -35,12 +35,12 @@ and [escape hatches][readme-escape-hatches].

## Navigation Item

The `NavigationItem` is a container for navigation links.
The `NavigationItem` is a container for navigation actions.

```jsx
import { NavigationItem } from '@lmc-eu/spirit-web-react';

<NavigationItem>{/* Navigation links go here */}</NavigationItem>;
<NavigationItem>{/* Navigation actions go here */}</NavigationItem>;
```

### API
Expand All @@ -53,64 +53,64 @@ The components accept [additional attributes][readme-additional-attributes].
If you need more control over the styling of a component, you can use [style props][readme-style-props]
and [escape hatches][readme-escape-hatches].

## Navigation Link
## Navigation Action

The `NavigationLink` is component that is styled to be used as a navigation link.
The `NavigationAction` is component that is styled to be used as a navigation action.

```jsx
import { NavigationLink } from '@lmc-eu/spirit-web-react';
import { NavigationAction } from '@lmc-eu/spirit-web-react';

<NavigationLink href="#">Link</NavigationLink>;
<NavigationAction href="#">Link</NavigationAction>;
```

It can obtain `isSelected` or `isDisabled` states by adding the respective props.

```jsx
<NavigationLink href="#" aria-current="page" isSelected>Selected Link</NavigationLink>
<NavigationLink href="#" isDisabled>Disabled Link</NavigationLink>
<NavigationAction href="#" aria-current="page" isSelected>Selected Link</NavigationAction>
<NavigationAction href="#" isDisabled>Disabled Link</NavigationAction>
```

ℹ️ Don't forget to add the `aria-current="page"` attribute for correct accessible state if selected.

ℹ️ Please note that in the `isDisabled` state the `NavigationLink` will be an `span` tag.
ℹ️ Please note that in the `isDisabled` state the `NavigationAction` will be a `span` tag.

If the `NavigationLink` is inside a [`UNSTABLE_Header`][web-react-unstable-header] component, it will
If the `NavigationAction` is inside a [`UNSTABLE_Header`][web-react-unstable-header] component, it will
inherit the height of the `Header`.

### API

| Name | Type | Default | Required | Description |
| ------------- | --------------------------------- | ------- | -------- | ----------------------------- |
| `children` | `string` \| `ReactNode` | `null` | ✓ | Content of the NavigationLink |
| `elementType` | `ElementType` | `a` | ✕ | Type of element used as |
| `href` | `string` | - | ✕ | URL of the link |
| `isDisabled` | `boolean` | `false` | ✕ | Whether the link is disabled |
| `isSelected` | `boolean` | `false` | ✕ | Whether the link is selected |
| `ref` | `ForwardedRef<HTMLAnchorElement>` | — | ✕ | Anchor element reference |
| `target` | `string` | `null` | ✕ | Link target |
| Name | Type | Default | Required | Description |
| ------------- | --------------------------------- | ------- | -------- | ------------------------------- |
| `children` | `string` \| `ReactNode` | `null` | ✓ | Content of the NavigationAction |
| `elementType` | `ElementType` | `a` | ✕ | Type of element used as |
| `href` | `string` | - | ✕ | URL of the link |
| `isDisabled` | `boolean` | `false` | ✕ | Whether the action is disabled |
| `isSelected` | `boolean` | `false` | ✕ | Whether the action is selected |
| `ref` | `ForwardedRef<HTMLAnchorElement>` | — | ✕ | Anchor element reference |
| `target` | `string` | `null` | ✕ | Link target |

The components accept [additional attributes][readme-additional-attributes].
If you need more control over the styling of a component, you can use [style props][readme-style-props]
and [escape hatches][readme-escape-hatches].

### Full Example

With NavigationLink components:
With NavigationAction components:

```jsx
<Navigation aria-label="Main Navigation">
<NavigationItem>
<NavigationLink href="#" aria-current="page" isSelected>
<NavigationAction href="#" aria-current="page" isSelected>
Selected Link
</NavigationLink>
</NavigationAction>
</NavigationItem>
<NavigationItem>
<NavigationLink href="#" isDisabled>
<NavigationAction href="#" isDisabled>
Disabled Link
</NavigationLink>
</NavigationAction>
</NavigationItem>
<NavigationItem>
<NavigationLink href="#">Link</NavigationLink>
<NavigationAction href="#">Link</NavigationAction>
</NavigationItem>
</Navigation>
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,45 +4,45 @@ import React from 'react';
import { classNamePrefixProviderTest } from '../../../../tests/providerTests/classNamePrefixProviderTest';
import { restPropsTest } from '../../../../tests/providerTests/restPropsTest';
import { stylePropsTest } from '../../../../tests/providerTests/stylePropsTest';
import NavigationLink from '../NavigationLink';
import NavigationAction from '../NavigationAction';

describe('NavigationLink', () => {
classNamePrefixProviderTest(NavigationLink, 'NavigationLink');
describe('NavigationAction', () => {
classNamePrefixProviderTest(NavigationAction, 'NavigationAction');

stylePropsTest(NavigationLink);
stylePropsTest(NavigationAction);

restPropsTest(NavigationLink, 'a');
restPropsTest(NavigationAction, 'a');

it('should have default classname', () => {
render(<NavigationLink href="/">Content</NavigationLink>);
render(<NavigationAction href="/">Content</NavigationAction>);

expect(screen.getByRole('link')).toHaveClass('NavigationLink');
expect(screen.getByRole('link')).toHaveClass('NavigationAction');
});

it('should have selected classname', () => {
render(
<NavigationLink href="/" isSelected>
<NavigationAction href="/" isSelected>
Content
</NavigationLink>,
</NavigationAction>,
);

expect(screen.getByRole('link')).toHaveClass('NavigationLink NavigationLink--selected');
expect(screen.getByRole('link')).toHaveClass('NavigationAction NavigationAction--selected');
});

it('should have disabled classname and correct elementType', () => {
render(
<NavigationLink href="/" isDisabled>
<NavigationAction href="/" isDisabled>
Content
</NavigationLink>,
</NavigationAction>,
);

expect(screen.getByText('Content')).toHaveClass('NavigationLink NavigationLink--disabled');
expect(screen.getByText('Content')).toHaveClass('NavigationAction NavigationAction--disabled');
expect(screen.getByText('Content')).toContainHTML('span');
expect(screen.queryByRole('link')).not.toBeInTheDocument();
});

it('should render children', () => {
render(<NavigationLink href="/">Content</NavigationLink>);
render(<NavigationAction href="/">Content</NavigationAction>);

expect(screen.getByText('Content')).toBeInTheDocument();
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { renderHook } from '@testing-library/react';
import { SpiritNavigationLinkProps } from '../../../types';
import { useNavigationLinkProps } from '../useNavigationLinkProps';
import { SpiritNavigationActionProps } from '../../../types';
import { useNavigationActionProps } from '../useNavigationActionProps';

describe('useNavigationLinkProps', () => {
describe('useNavigationActionProps', () => {
it('should return defaults props', () => {
const props: SpiritNavigationLinkProps = {
const props: SpiritNavigationActionProps = {
href: '/',
target: '_blank',
isSelected: false,
isDisabled: false,
};
const { result } = renderHook(() => useNavigationLinkProps(props));
const { result } = renderHook(() => useNavigationActionProps(props));

expect(result.current).toStrictEqual({
navigationLinkProps: {
navigationActionProps: {
href: '/',
rel: undefined,
target: '_blank',
Expand All @@ -22,17 +22,17 @@ describe('useNavigationLinkProps', () => {
});

it('should return defaults if element is different from anchor', () => {
const props: SpiritNavigationLinkProps<'span'> = {
const props: SpiritNavigationActionProps<'span'> = {
elementType: 'span',
href: '/',
target: '_blank',
isSelected: false,
isDisabled: false,
};
const { result } = renderHook(() => useNavigationLinkProps(props as SpiritNavigationLinkProps));
const { result } = renderHook(() => useNavigationActionProps(props as SpiritNavigationActionProps));

expect(result.current).toStrictEqual({
navigationLinkProps: {
navigationActionProps: {
href: undefined,
rel: undefined,
target: undefined,
Expand All @@ -41,16 +41,16 @@ describe('useNavigationLinkProps', () => {
});

it('should return for isDisabled', () => {
const props: SpiritNavigationLinkProps = {
const props: SpiritNavigationActionProps = {
href: '/',
target: '_blank',
isSelected: false,
isDisabled: true,
};
const { result } = renderHook(() => useNavigationLinkProps(props));
const { result } = renderHook(() => useNavigationActionProps(props));

expect(result.current).toStrictEqual({
navigationLinkProps: {
navigationActionProps: {
href: undefined,
rel: undefined,
target: undefined,
Expand All @@ -59,16 +59,16 @@ describe('useNavigationLinkProps', () => {
});

it('should return for isSelected', () => {
const props: SpiritNavigationLinkProps = {
const props: SpiritNavigationActionProps = {
href: '/',
target: '_blank',
isSelected: true,
isDisabled: false,
};
const { result } = renderHook(() => useNavigationLinkProps(props));
const { result } = renderHook(() => useNavigationActionProps(props));

expect(result.current).toStrictEqual({
navigationLinkProps: {
navigationActionProps: {
href: '/',
rel: undefined,
target: '_blank',
Expand All @@ -77,17 +77,17 @@ describe('useNavigationLinkProps', () => {
});

it('should return for elementType', () => {
const props: SpiritNavigationLinkProps<'div'> = {
const props: SpiritNavigationActionProps<'div'> = {
elementType: 'div',
crishpeen marked this conversation as resolved.
Show resolved Hide resolved
href: '/',
target: '_blank',
isSelected: false,
isDisabled: false,
};
const { result } = renderHook(() => useNavigationLinkProps(props as SpiritNavigationLinkProps));
const { result } = renderHook(() => useNavigationActionProps(props as SpiritNavigationActionProps));

expect(result.current).toStrictEqual({
navigationLinkProps: {
navigationActionProps: {
href: undefined,
rel: undefined,
target: undefined,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { renderHook } from '@testing-library/react';
import { SpiritNavigationActionProps } from '../../../types';
import { useNavigationActionStyleProps } from '../useNavigationActionStyleProps';

describe('useNavigationActionStyleProps', () => {
it('should return defaults', () => {
const props = {};
const { result } = renderHook(() => useNavigationActionStyleProps(props));

expect(result.current.classProps).toBe('NavigationAction');
});

it('should return disabled class', () => {
const props: SpiritNavigationActionProps = { isDisabled: true };
const { result } = renderHook(() => useNavigationActionStyleProps(props));

expect(result.current.classProps).toBe('NavigationAction NavigationAction--disabled');
});

it('should return selected class', () => {
const props = { isSelected: true };
const { result } = renderHook(() => useNavigationActionStyleProps(props));

expect(result.current.classProps).toBe('NavigationAction NavigationAction--selected');
});
});
Loading
Loading