Skip to content
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 @@ -15,9 +15,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* @flow strict */

import BpkLabel from '../../packages/bpk-component-label';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import { BpkDarkExampleWrapper } from '../bpk-storybook-utils';

const DefaultExample = () => <BpkLabel htmlFor="origin">Origin</BpkLabel>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ export const InvalidRequired = InvalidRequiredExample;
export const White = WhiteExample;

export const VisualTest = MixedExample;
export const VisualTestWithZoom = VisualTest.bind({});
VisualTestWithZoom.args = {
zoomEnabled: true
export const VisualTestWithZoom = {
render: VisualTest,
args: {
zoomEnabled: true,
},
};
1 change: 0 additions & 1 deletion examples/bpk-component-textarea/examples.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
* limitations under the License.
*/

// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import BpkLabel from '../../packages/bpk-component-label';
import BpkTextarea from '../../packages/bpk-component-textarea';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
Expand Down
1 change: 0 additions & 1 deletion packages/bpk-component-fieldset/src/BpkFieldset.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type { ReactElement } from 'react';

// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import BpkFormValidation from '../../bpk-component-form-validation';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import BpkLabel from '../../bpk-component-label';
import { cssModules } from '../../bpk-react-utils';

Expand Down
2 changes: 1 addition & 1 deletion packages/bpk-component-label/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Check the main [Readme](https://github.com/skyscanner/backpack#usage) for a comp

## Usage

```js
```tsx
import BpkLabel from '@skyscanner/backpack-web/bpk-component-label';

export default () => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* @flow strict */

import BpkLabel, { type Props } from './src/BpkLabel';
import BpkLabel from './src/BpkLabel';

export type BpkLabelProps = Props;
export type { Props as BpkLabelProps } from './src/BpkLabel';

export default BpkLabel;
67 changes: 0 additions & 67 deletions packages/bpk-component-label/src/BpkLabel-test.js

This file was deleted.

108 changes: 108 additions & 0 deletions packages/bpk-component-label/src/BpkLabel-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Backpack - Skyscanner's Design System
*
* Copyright 2016 Skyscanner Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { render } from '@testing-library/react';
import '@testing-library/jest-dom';

import BpkLabel from './BpkLabel';

describe('BpkLabel', () => {
it('should render correctly', () => {
const { container } = render(<BpkLabel>Origin</BpkLabel>);
const label = container.querySelector('label');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveTextContent('Origin');
expect(label?.querySelector('.bpk-label__asterisk')).not.toBeInTheDocument();
});

it('should render correctly with a "white" attribute', () => {
const { container } = render(<BpkLabel white>Origin</BpkLabel>);
const label = container.querySelector('label');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveClass('bpk-label--white');
expect(label).toHaveTextContent('Origin');
});

it('should render correctly with a "required" attribute', () => {
const { container } = render(<BpkLabel required>Origin</BpkLabel>);
const label = container.querySelector('label');
const asterisk = label?.querySelector('.bpk-label__asterisk');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveTextContent('Origin');
expect(asterisk).toBeInTheDocument();
expect(asterisk).toHaveTextContent('*');
});

it('should render correctly with "valid" attribute false', () => {
const { container } = render(<BpkLabel valid={false}>Origin</BpkLabel>);
const label = container.querySelector('label');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveClass('bpk-label--invalid');
expect(label).toHaveTextContent('Origin');
});

it('should render correctly with "valid" attribute false and "required" attributes', () => {
const { container } = render(
<BpkLabel required valid={false}>
Origin
</BpkLabel>,
);
const label = container.querySelector('label');
const asterisk = label?.querySelector('.bpk-label__asterisk');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveClass('bpk-label--invalid');
expect(label).toHaveTextContent('Origin');
expect(asterisk).toBeInTheDocument();
expect(asterisk).toHaveTextContent('*');
});

it('should render correctly without an asterisk when disabled and required', () => {
const { container } = render(
<BpkLabel disabled required>
Origin
</BpkLabel>,
);
const label = container.querySelector('label');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveClass('bpk-label--disabled');
expect(label).toHaveTextContent('Origin');
expect(label?.querySelector('.bpk-label__asterisk')).not.toBeInTheDocument();
});

it('should render correctly with a "className" attribute', () => {
const { container } = render(<BpkLabel className="test">Origin</BpkLabel>);
const label = container.querySelector('label');

expect(label).toBeInTheDocument();
expect(label).toHaveClass('bpk-label');
expect(label).toHaveClass('test');
expect(label).toHaveTextContent('Origin');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* @flow strict */

import PropTypes from 'prop-types';
import type { Node } from 'react';
import type { ComponentPropsWithoutRef, ReactNode } from 'react';

import { cssModules } from '../../bpk-react-utils';

Expand All @@ -27,17 +25,17 @@ import STYLES from './BpkLabel.module.scss';
const getClassName = cssModules(STYLES);

export type Props = {
children: Node,
className: ?string,
disabled: boolean,
valid: ?boolean,
required: boolean,
white: boolean,
};
children: ReactNode;
className?: string;
disabled?: boolean;
valid?: boolean | null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The valid is supposed to be a boolean and only has 2 values, but it requires changing the code below. To keep the TS migration pure and safe, keep it as it was for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is nowhere use null value for valid, and in L45 only handle false value without null and undefined, should it clearly distinguish between null and undefined?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ezreal (@Ezreal-Yang), thanks for asking! I have considered this question. Currently, the valid property has three values: true, false, and null, which seems unnecessary. In my opinion, it only needs two values to control its style effectively. It should display the invalid style only when valid is set to false.

I'm unsure how this will be used downstream, especially in cases where valid might be set to null. If I modify it directly, it could lead to a type error in its usage. As I mentioned in my comment, I prefer to keep it as it is in this PR.

required?: boolean;
white?: boolean;
} & ComponentPropsWithoutRef<'label'>;

const BpkLabel = ({
children,
className = null,
className,
disabled = false,
required = false,
valid = null,
Expand All @@ -56,7 +54,6 @@ const BpkLabel = ({
);

return (
// $FlowFixMe[cannot-spread-inexact] - inexact rest. See 'decisions/flowfixme.md'.
<label className={classNames} {...rest}>
{children}
{!disabled && required && (
Expand All @@ -66,15 +63,4 @@ const BpkLabel = ({
);
};

BpkLabel.propTypes = {
children: PropTypes.node.isRequired,
className: PropTypes.string,
disabled: PropTypes.bool,
valid: PropTypes.bool,
required: PropTypes.bool,
white: PropTypes.bool,
};



export default BpkLabel;

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* @flow strict */

import { render } from '@testing-library/react';
import { axe } from 'jest-axe';
Expand Down
1 change: 0 additions & 1 deletion packages/bpk-component-nudger/src/BpkNudger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import BpkButton, { BUTTON_TYPES } from '../../bpk-component-button';
import { withButtonAlignment } from '../../bpk-component-icon';
import MinusIcon from '../../bpk-component-icon/sm/minus';
import PlusIcon from '../../bpk-component-icon/sm/plus';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import BpkLabel from '../../bpk-component-label';
import BpkText, { TEXT_STYLES } from '../../bpk-component-text';
import { cssModules, setNativeValue } from '../../bpk-react-utils';
Expand Down
Loading
Loading