Skip to content

Commit

Permalink
fix(dropdown): react - dont pass hidden prop to cds-dropdown when false
Browse files Browse the repository at this point in the history
Fixes  #151

passing in a false value renders hidden="false" in the DOM, which is breaking the overlay layer logic

This is the boolean attribute issue that also effected modals as well
  • Loading branch information
Ashley Ryan authored and ashleyryan committed Aug 24, 2022
1 parent 7a69762 commit 41198b3
Show file tree
Hide file tree
Showing 9 changed files with 14,578 additions and 80 deletions.
14,480 changes: 14,465 additions & 15 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion projects/react/package.json
Expand Up @@ -21,7 +21,7 @@
"@testing-library/dom": "^8.11.3",
"@testing-library/jest-dom": "^5.16.2",
"@testing-library/react": "^12.1.2",
"@testing-library/user-event": "^13.5.0",
"@testing-library/user-event": "^14.4.3",
"@types/jest": "^27.4.0",
"@types/node": "^14.0.5",
"@types/react": "^17.0.38",
Expand Down
30 changes: 27 additions & 3 deletions projects/react/src/dropdown/index.test.tsx
@@ -1,14 +1,17 @@
import * as React from 'react';
import { render } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { CdsDropdown } from './index';

const TestComponent = () => {
const TestComponent = ({ defaultHidden = false }: { defaultHidden?: boolean }) => {
const [hidden, setHidden] = React.useState(defaultHidden);
const ref = React.useRef<HTMLSpanElement>();

return (
<>
<button onClick={() => setHidden(false)} />
<span ref={ref}>Anchor Element</span>
<CdsDropdown anchor={ref.current}></CdsDropdown>
<CdsDropdown hidden={hidden} anchor={ref.current} onCloseChange={() => setHidden(true)}></CdsDropdown>
</>
);
};
Expand All @@ -19,6 +22,27 @@ describe('CdsDropdown', () => {
render(<TestComponent></TestComponent>);
});

it('should open and close', async () => {
const user = userEvent.setup();
render(<TestComponent defaultHidden></TestComponent>);

const dropdown = document.querySelector('cds-dropdown');
expect(dropdown).toHaveAttribute('hidden', 'true');
expect(await screen.queryByRole('dialog')).not.toBeInTheDocument();

await user.click(await screen.findByRole('button'));
expect(dropdown).not.toHaveAttribute('hidden');
expect(await screen.queryByRole('dialog')).toBeInTheDocument();

// clicking the backdrop should hide the dropdown
const overlay = dropdown.shadowRoot.querySelector('.overlay-backdrop');
expect(overlay).not.toBeUndefined();

await user.click(overlay);
expect(dropdown).toHaveAttribute('hidden', 'true');
expect(await screen.queryByRole('dialog')).not.toBeInTheDocument();
});

it('snapshot', () => {
const vertDivStyle = {
height: '140px',
Expand Down
22 changes: 12 additions & 10 deletions projects/react/src/dropdown/index.tsx
Expand Up @@ -2,17 +2,19 @@ import { CdsDropdown as Dropdown } from '@cds/core/dropdown';
import '@cds/core/dropdown/register';
import { createComponent } from '@lit-labs/react';
import * as React from 'react';
import { logReactVersion } from '../utils/index.js';
import { logReactVersion, removeFalseHiddenProp } from '../utils/index.js';

export const CdsDropdown = createComponent(
React,
'cds-dropdown',
Dropdown,
{
onCloseChange: 'closeChange',
onCdsMotionChange: 'cdsMotionChange',
},
'CdsDropdown'
export const CdsDropdown = removeFalseHiddenProp(
createComponent(
React,
'cds-dropdown',
Dropdown,
{
onCloseChange: 'closeChange',
onCdsMotionChange: 'cdsMotionChange',
},
'CdsDropdown'
)
);

logReactVersion(React);
36 changes: 27 additions & 9 deletions projects/react/src/modal/index.test.tsx
@@ -1,5 +1,6 @@
import * as React from 'react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { CdsModal, CdsModalActions, CdsModalContent, CdsModalHeader } from './index';

describe('CdsModal', () => {
Expand Down Expand Up @@ -27,16 +28,33 @@ describe('CdsModal', () => {
expect(document.querySelector('cds-modal')).not.toHaveAttribute('hidden');
});

it('has attribute hidden when hidden', () => {
render(
<CdsModal hidden>
<CdsModalHeader>
<h3 cds-text="title">My Modal</h3>
</CdsModalHeader>
</CdsModal>
);
it('should open and close', async () => {
const user = userEvent.setup();
const TestComponent = () => {
const [hidden, setHidden] = React.useState(true);
return (
<>
<button onClick={() => setHidden(false)}></button>
<CdsModal hidden={hidden} onCloseChange={() => setHidden(true)}>
<CdsModalHeader>
<h3 cds-text="title">My Modal</h3>
</CdsModalHeader>
</CdsModal>
</>
);
};

render(<TestComponent></TestComponent>);

const modal = document.querySelector('cds-modal');
expect(modal).toHaveAttribute('hidden', 'true');

await user.click(screen.getByRole('button'));
expect(modal).not.toHaveAttribute('hidden');

expect(document.querySelector('cds-modal')).toHaveAttribute('hidden', 'true');
// clicking the backdrop should hide the modal
await user.click(modal.shadowRoot.querySelector('.overlay-backdrop'));
expect(modal).toHaveAttribute('hidden', 'true');
});

it('snapshot', () => {
Expand Down
22 changes: 1 addition & 21 deletions projects/react/src/modal/index.tsx
Expand Up @@ -8,27 +8,7 @@ import {
import '@cds/core/modal/register';
import { createComponent } from '@lit-labs/react';
import * as React from 'react';
import { logReactVersion } from '../utils/index.js';

/**
* Converts a false value for `hidden` to `undefined` so it doesn't get rendered in the DOM
*
* There is an open issue where lit/react passes the false value to the element in the dom
* this isn't valid per the spec for boolean attributes.
*
* We mostly work around it with CSS, but it's still causing some problems with the modal overlay
* REMOVE THIS WHEN THE FOLLOWING ISSUE IS RESOLVED (also remove all of the CSS fixes)
*
* https://github.com/lit/lit/issues/3053
* @param CdsButton
* @returns CdsButton
*/
function removeFalseHiddenProp<P extends { hidden?: boolean }>(Component: React.ComponentType<P>) {
return (props: P) => {
const { hidden } = props;
return <Component {...(props as P)} hidden={hidden ? true : undefined} />;
};
}
import { removeFalseHiddenProp, logReactVersion } from '../utils/index.js';

export const CdsModal = removeFalseHiddenProp(
createComponent(
Expand Down
22 changes: 12 additions & 10 deletions projects/react/src/signpost/index.tsx
Expand Up @@ -2,17 +2,19 @@ import { CdsSignpost as Signpost } from '@cds/core/signpost';
import '@cds/core/signpost/register';
import { createComponent } from '@lit-labs/react';
import * as React from 'react';
import { logReactVersion } from '../utils/index.js';
import { logReactVersion, removeFalseHiddenProp } from '../utils/index.js';

export const CdsSignpost = createComponent(
React,
'cds-signpost',
Signpost,
{
onCloseChange: 'closeChange',
onCdsMotionChange: 'cdsMotionChange',
},
'CdsSignpost'
export const CdsSignpost = removeFalseHiddenProp(
createComponent(
React,
'cds-signpost',
Signpost,
{
onCloseChange: 'closeChange',
onCdsMotionChange: 'cdsMotionChange',
},
'CdsSignpost'
)
);

logReactVersion(React);
11 changes: 0 additions & 11 deletions projects/react/src/utils/index.ts

This file was deleted.

33 changes: 33 additions & 0 deletions projects/react/src/utils/index.tsx
@@ -0,0 +1,33 @@
/*
* Copyright (c) 2016-2022 VMware, Inc. All Rights Reserved.
* This software is released under MIT license.
* The full license information can be found in LICENSE in the root directory of this project.
*/

import React from 'react';

export function logReactVersion(react: { version: string }) {
if (window && (window as any).CDS && !(window as any).CDS._react.version) {
(window as any).CDS._react.version = react.version;
}
}

/**
* Converts a false value for `hidden` to `undefined` so it doesn't get rendered in the DOM
*
* There is an open issue where lit/react passes the false value to the element in the dom
* this isn't valid per the spec for boolean attributes.
*
* We mostly work around it with CSS, but it's still causing some problems with the modal overlay
* REMOVE THIS WHEN THE FOLLOWING ISSUE IS RESOLVED (also remove all of the CSS fixes)
*
* https://github.com/lit/lit/issues/3053
* @param Component
* @returns Component
*/
export function removeFalseHiddenProp<P extends { hidden?: boolean }>(Component: React.ComponentType<P>) {
return (props: P) => {
const { hidden } = props;
return <Component {...(props as P)} hidden={hidden ? true : undefined} />;
};
}

0 comments on commit 41198b3

Please sign in to comment.