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

feat: move Loader to compat package & replace with BusyIndicator #6020

Merged
merged 11 commits into from
Jul 5, 2024
48 changes: 48 additions & 0 deletions docs/MigrationGuide.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,54 @@ function MyComponent() {
}
```

### Loader

There is no longer a concept of a Loader component defined by the UX guidelines. To indicate a loading state, it is now recommended using the `BusyIndicator` instead.
For backwards compatibility, the Loader is still available in the `@ui5/webcomponents-react-compat` package, but it may lack accessibility features and no longer receives feature updates.

#### Replacing `Loader` with `BusyIndicator`

Unfortunately, there is no general rule of thumb for how to replace the `Loader` component with the `BusyIndicator` component. In most cases it should be sufficient wrapping your component inside the `BusyIndicator`, as shown below:

```jsx
// v1

<Card header={<CardHeader titleText="Card Header" />}>
<Loader />
<div style={{ height: '400px', padding: '1rem' }}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer sed felis tristique, molestie tellus id, rutrum
urna. Quisque mattis risus imperdiet gravida accumsan. Proin elementum efficitur diam eu interdum.
</div>
</Card>

// v2

<Card header={<CardHeader titleText="Card Header" />}>
<BusyIndicator active delay={0}>
<div style={{ height: '400px', padding: '1rem' }}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer sed felis tristique, molestie tellus id,
rutrum urna. Quisque mattis risus imperdiet gravida accumsan. Proin elementum efficitur diam eu interdum.
</div>
</BusyIndicator>
</Card>
```

However, for components that apply complex styles such as `absolute/fixed` positioning, this might not be the case, as the `BusyIndicator` brings its own set of styles.
In such instances, we recommend positioning the `BusyIndicator` above the element that should receive a loading indicator e.g. via `position: absolute`.
If you encounter any issues migrating to the `BusyIndicator`, please feel free to reach out via [GitHub Discussions](https://github.com/SAP/ui5-webcomponents-react/discussions) or create an [Issue](https://github.com/SAP/ui5-webcomponents-react/issues/new/choose) if the behavior seems like a bug.

#### Keep the `Loader`

If you'd like to keep the `Loader` component instead of the `BusyIndicator` component, you now need to include the `@ui5/webcomponents-react-compat` package in your dependencies and adjust all import paths accordingly:

```ts
// v1
import { Loader } from '@ui5/webcomponents-react';

// v2
import { Loader } from '@ui5/webcomponents-react-compat';
```

### ObjectPage

The newly introduced `DynamicPage` web component comes with its own `DynamicPageHeader` and `DynamicPageTitle` components, which are unfortunately incompatible with our `ObjectPage` implementation.
Expand Down
10 changes: 9 additions & 1 deletion packages/charts/src/internal/ChartContainer.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,19 @@
color: var(--sapTextColor);
font-family: var(--sapFontFamily);
width: 100%;
/*todo remove in v2*/
height: 400px;
min-height: fit-content;
position: relative;
}

.busyIndicator {
background-color: color-mix(in srgb, var(--sapBackgroundColor) 45%, transparent);
position: absolute;
inset: 0;
height: 100%;
z-index: 1;
}

:global(.has-click-handler) {
:global(.recharts-pie-sector),
:global(.recharts-bar-rectangles),
Expand Down
34 changes: 24 additions & 10 deletions packages/charts/src/internal/ChartContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
import { type CommonProps, Label, Loader } from '@ui5/webcomponents-react';
import { BusyIndicator, Label } from '@ui5/webcomponents-react';
import type { CommonProps } from '@ui5/webcomponents-react';
import { addCustomCSSWithScoping } from '@ui5/webcomponents-react/dist/internal/addCustomCSSWithScoping.js';
import { useStylesheet } from '@ui5/webcomponents-react-base';
import { clsx } from 'clsx';
import type { ComponentType, CSSProperties, ReactElement, ReactNode } from 'react';
import type { ComponentType, ReactElement, ReactNode } from 'react';
import { Component, forwardRef } from 'react';
import { ResponsiveContainer } from 'recharts';
import { classNames, styleData } from './ChartContainer.module.css.js';

//todo: add feature request for parts or even a fix if this turns out to be a bug
addCustomCSSWithScoping(
'ui5-busy-indicator',
`
:host([data-component-name="ChartContainerBusyIndicator"]) .ui5-busy-indicator-busy-area{
background:unset;
},
:host([data-component-name="ChartContainerBusyIndicator"]) .ui5-busy-indicator-busy-area:focus {
border-radius: 0;
}
`
);

export interface ContainerProps extends CommonProps {
children: ReactElement;
Placeholder?: ComponentType;
Expand All @@ -14,13 +29,6 @@ export interface ContainerProps extends CommonProps {
resizeDebounce: number;
}

const loaderStyles: CSSProperties = {
position: 'absolute',
top: 0,
left: 0,
right: 0
};

class ErrorBoundary extends Component<{ children: ReactNode }, { errorCount: number }> {
state = {
errorCount: 0
Expand Down Expand Up @@ -49,7 +57,13 @@ const ChartContainer = forwardRef<HTMLDivElement, ContainerProps>((props, ref) =
<div ref={ref} className={clsx(classNames.container, className)} slot={slot} {...rest}>
{dataset?.length > 0 ? (
<>
{loading && <Loader style={loaderStyles} />}
{loading && (
<BusyIndicator
active
className={classNames.busyIndicator}
data-component-name="ChartContainerBusyIndicator"
/>
)}
<ErrorBoundary>
<ResponsiveContainer debounce={resizeDebounce}>{children}</ResponsiveContainer>
</ErrorBoundary>
Expand Down
7 changes: 3 additions & 4 deletions packages/compat/src/components/Loader/Loader.cy.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { LoaderType } from '../../enums/index.js';
import { Loader } from './index.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import type { LoaderType } from '@/packages/main';
import { Loader } from '@/packages/main';

// skip until component is moved to this package
describe.skip('Loader', () => {
describe('Loader', () => {
it('indeterminate', () => {
cy.mount(<Loader data-testid="loader" />);
cy.findByTestId('loader').should('have.css', 'animation-duration', '1.2s');
Expand Down
42 changes: 40 additions & 2 deletions packages/compat/src/components/Loader/Loader.module.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,41 @@
.dummy {
background-color: red;
.loader {
position: relative;
height: 0.25rem;
width: 100%;

&:before {
content: '';
position: absolute;
left: 0;
width: 100%;
height: 100%;
background-color: var(--sapContent_BusyColor);
opacity: 0.15;
}

&.loaderDeterminate {
background: linear-gradient(to right, var(--sapContent_BusyColor), var(--sapContent_BusyColor)) repeat-y;
}

&.loaderIndeterminate {
background-size: 40%;
background: linear-gradient(
to right,
transparent 0px,
var(--sapContent_BusyColor) calc(50% - 2rem),
var(--sapContent_BusyColor) calc(50% + 2rem),
transparent 100%
)
repeat-y;
animation: scroll 1.2s linear infinite;
}
}

@keyframes scroll {
0% {
background-position: -100% 0;
}
100% {
background-position: 200% 0;
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import type { Meta, StoryObj } from '@storybook/react';
import activateIcon from '@ui5/webcomponents-icons/dist/activate.js';
import { Card, CardHeader, FlexBox, FlexBoxDirection, Icon, Text } from '@ui5/webcomponents-react';
import { useEffect, useRef, useState } from 'react';
import { FlexBoxDirection, LoaderType } from '../../enums/index.js';
import { Card } from '../../webComponents/Card/index.js';
import { CardHeader } from '../../webComponents/CardHeader/index.js';
import { Icon } from '../../webComponents/Icon/index.js';
import { Text } from '../../webComponents/Text/index.js';
import { FlexBox } from '../FlexBox/index.js';
import { LoaderType } from '../../enums/LoaderType.js';
import { Loader } from './index.js';

const meta = {
title: 'User Feedback / Loader',
title: 'Loader',
component: Loader,
argTypes: {},
args: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
'use client';

import { deprecationNotice, useI18nBundle, useStylesheet } from '@ui5/webcomponents-react-base';
import type { CommonProps } from '@ui5/webcomponents-react';
import { PLEASE_WAIT } from '@ui5/webcomponents-react/dist/i18n/i18n-defaults.js';
import { useI18nBundle, useStylesheet } from '@ui5/webcomponents-react-base';
import { clsx } from 'clsx';
import type { CSSProperties } from 'react';
import { forwardRef, useEffect, useState } from 'react';
import { LoaderType } from '../../enums/index.js';
import { PLEASE_WAIT } from '../../i18n/i18n-defaults.js';
import type { CommonProps } from '../../types/index.js';
import { LoaderType } from '../../enums/LoaderType.js';
import { classNames, styleData } from './Loader.module.css.js';

export interface LoaderPropTypes extends CommonProps {
Expand Down Expand Up @@ -35,12 +35,10 @@ export interface LoaderPropTypes extends CommonProps {
}

/**
* The `Loader` signals that an operation is currently being executed. It uses as little space as possible to allow the user to interact with the UI.<br />
* It can be used to signal a data update on an already existing dataset, or where an expansion will happen.
*
* __Note:__ This component is __deprecated__ and will be removed with our next major release (v2.0.0)! Please use the [BusyIndicator](https://sap.github.io/ui5-webcomponents-react/?path=/docs/user-feedback-busyindicator--docs) instead.
* __Note__: There is no longer a concept of a Loader component defined by the UX guidelines! To indicate a loading state, please use the `BusyIndicator` instead. For backwards compatibility, the Loader is still available in the `@ui5/webcomponents-react-compat` package, but it may lack accessibility features and no longer receives feature updates.
*
* @deprecated This component is deprecated and will be removed with our next major release (v2.0.0)! Please use the [BusyIndicator](https://sap.github.io/ui5-webcomponents-react/?path=/docs/user-feedback-busyindicator--docs) instead.
* The `Loader` signals that an operation is currently being executed. It uses as little space as possible to allow the user to interact with the UI.
* It can be used to signal a data update on an already existing dataset, or where an expansion will happen.
*/
const Loader = forwardRef<HTMLDivElement, LoaderPropTypes>((props, ref) => {
const { className, type = LoaderType.Indeterminate, progress = '0px', slot, style, delay = 0, ...rest } = props;
Expand All @@ -51,10 +49,6 @@ const Loader = forwardRef<HTMLDivElement, LoaderPropTypes>((props, ref) => {
const loaderClasses = clsx(classNames.loader, className, classNames[`loader${type}`]);
const backgroundSize = type !== LoaderType.Determinate ? '40%' : progress;

useEffect(() => {
deprecationNotice('Loader', 'The `Loader` component is deprecated. Please use the `BusyIndicator` instead.');
}, []);

useEffect(() => {
let timeout;
if (delay > 0) {
Expand Down
3 changes: 3 additions & 0 deletions packages/compat/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ export * from './components/TableCell/index.js';
export * from './components/TableColumn/index.js';
export * from './components/TableGroupRow/index.js';
export * from './components/TableRow/index.js';
export * from './components/Loader/index.js';

export { LoaderType } from './enums/LoaderType.js';
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ describe('AnalyticalTable', () => {
cy.mount(<AnalyticalTable data={[]} columns={columns} loading />);
cy.get('[data-component-name="AnalyticalTableLoadingPlaceholder"]').should('be.visible');
cy.mount(<AnalyticalTable data={data} columns={columns} loading />);
cy.get('[data-component-name="Loader"]').should('be.visible');
cy.get('[data-component-name="AnalyticalTableBusyIndicator"]').should('be.visible');
cy.mount(<AnalyticalTable data={[]} columns={columns} />);
cy.findByText('No data').should('be.visible');
cy.mount(<AnalyticalTable data={data} columns={columns} filterable globalFilterValue="test123" />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ const meta = {
reactTableOptions: { control: { disable: true } },
tableHooks: { control: { disable: true } },
NoDataComponent: { control: { disable: true } },
LoadingComponent: { control: { disable: true } },
extension: { control: { disable: true } },
tableInstance: { control: { disable: true } },
portalContainer: { control: { disable: true } }
Expand Down

This file was deleted.

This file was deleted.

Loading
Loading