Skip to content

Commit

Permalink
fix(NativeSelect): Fixed related simple pagination issue with selecte…
Browse files Browse the repository at this point in the history 
…d states not updating after selection. (#1203)
  • Loading branch information
@chris-cedrone-cengage
chris-cedrone-cengage authored Apr 1, 2024
1 parent 280d9ba commit a869e09
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 57 deletions.
5 changes: 5 additions & 0 deletions .changeset/fix-NativeSelectPagination.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'react-magma-dom': patch
---

fix(NativeSelect): Fixes the issue with pagination control, rows per page, on Table and Datagrid per ticket: https://github.com/cengage/react-magma/issues/1201
51 changes: 50 additions & 1 deletion packages/react-magma-dom/src/components/NativeSelect/NativeSelect.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { axe } from '../../../axe-helper';
import { magma } from '../../theme/magma';
import { NativeSelect } from '.';
import { render } from '@testing-library/react';
import { render, fireEvent } from '@testing-library/react';
import { transparentize } from 'polished';
import { Tooltip } from '../Tooltip';
import { IconButton } from '../IconButton';
Expand Down Expand Up @@ -65,6 +65,21 @@ describe('NativeSelect', () => {
);
});

it('should update the selected option', () => {
const { getByTestId } = render(
<NativeSelect testId={testId}>
<option>1</option>
<option>2</option>
<option>3</option>
</NativeSelect>
);
const activeOption = getByTestId(testId);

fireEvent.change(getByTestId(testId), { target: { value: 2 } });

expect(activeOption).toHaveDisplayValue('2');
});

it('should render a default inverse border', () => {
const { getByTestId } = render(
<NativeSelect isInverse testId={testId}></NativeSelect>
Expand Down Expand Up @@ -159,5 +174,39 @@ describe('NativeSelect', () => {
'flex'
);
});

it(`Should display an additional wrapper with additionalContent'`, () => {
const { rerender, queryByTestId } = render(
<NativeSelect
labelPosition="left"
testId={testId}
additionalContent={
<Tooltip content={helpLinkLabel}>
<IconButton
aria-label={helpLinkLabel}
icon={<HelpIcon />}
onClick={onHelpLinkClick}
testId="Icon Button"
type={ButtonType.button}
size={ButtonSize.small}
variant={ButtonVariant.link}
/>
</Tooltip>
}
/>
);
expect(
queryByTestId(`${testId}-additional-content-wrapper`)
).toBeInTheDocument();
});

it(`Shouldn't display an additional wrapper without additionalContent'`, () => {
const { rerender, queryByTestId } = render(
<NativeSelect labelPosition="left" testId={testId} />
);
expect(
queryByTestId(`${testId}-additional-content-wrapper`)
).not.toBeInTheDocument();
});
});
});
115 changes: 63 additions & 52 deletions packages/react-magma-dom/src/components/NativeSelect/NativeSelect.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,71 +139,82 @@ export const NativeSelect = React.forwardRef<HTMLDivElement, NativeSelectProps>(

const hasLabel = !!labelText;

const nativeSelect = (
<StyledFormFieldContainer
additionalContent={additionalContent}
containerStyle={containerStyle}
testId={testId && `${testId}-form-field-container`}
errorMessage={errorMessage}
fieldId={id}
hasLabel={!!labelText}
labelPosition={labelPosition}
labelStyle={labelStyle}
labelText={
labelPosition !== LabelPosition.left && additionalContent ? (
<>
{labelText}
{labelText && additionalContent}
</>
) : (
labelText
)
}
labelWidth={labelWidth}
isInverse={isInverse}
helperMessage={helperMessage}
messageStyle={messageStyle}
ref={ref}
>
<StyledNativeSelectWrapper
disabled={disabled}
hasError={!!errorMessage}
isInverse={isInverse}
theme={theme}
>
<StyledNativeSelect
data-testid={testId}
hasError={!!errorMessage}
disabled={disabled}
id={id}
isInverse={isInverse}
theme={theme}
{...other}
>
{children}
</StyledNativeSelect>
<DefaultDropdownIndicator disabled={disabled} />
</StyledNativeSelectWrapper>
</StyledFormFieldContainer>
);

// If the labelPosition is set to 'left' then a <div> wraps the FormFieldContainer, NativeSelectWrapper, and NativeSelect for proper styling alignment.
function AdditionalContentWrapper(props) {
if (
labelPosition === LabelPosition.left ||
(labelPosition === LabelPosition.top && !hasLabel)
) {
return (
<StyledAdditionalContentWrapper theme={theme}>
<StyledAdditionalContentWrapper
data-testid={`${testId}-additional-content-wrapper`}
theme={theme}
>
{props.children}
</StyledAdditionalContentWrapper>
);
}
return props.children;
}

return (
<AdditionalContentWrapper labelPosition={labelPosition}>
<StyledFormFieldContainer
additionalContent={additionalContent}
containerStyle={containerStyle}
testId={testId && `${testId}-form-field-container`}
errorMessage={errorMessage}
fieldId={id}
hasLabel={!!labelText}
labelPosition={labelPosition}
labelStyle={labelStyle}
labelText={
labelPosition !== LabelPosition.left && additionalContent ? (
<>
{labelText}
{labelText && additionalContent}
</>
) : (
labelText
)
}
labelWidth={labelWidth}
isInverse={isInverse}
helperMessage={helperMessage}
messageStyle={messageStyle}
ref={ref}
>
<StyledNativeSelectWrapper
disabled={disabled}
hasError={!!errorMessage}
isInverse={isInverse}
theme={theme}
>
<StyledNativeSelect
data-testid={testId}
hasError={!!errorMessage}
disabled={disabled}
id={id}
isInverse={isInverse}
theme={theme}
{...other}
>
{children}
</StyledNativeSelect>
<DefaultDropdownIndicator disabled={disabled} />
</StyledNativeSelectWrapper>
</StyledFormFieldContainer>
{(labelPosition === 'left' && additionalContent) ||
(!labelText && additionalContent)}
</AdditionalContentWrapper>
);
if (additionalContent) {
return (
<AdditionalContentWrapper labelPosition={labelPosition}>
{nativeSelect}
{(labelPosition === LabelPosition.left && additionalContent) ||
(!labelText && additionalContent)}
</AdditionalContentWrapper>
);
} else {
return nativeSelect;
}
}
);
11 changes: 7 additions & 4 deletions packages/react-magma-dom/src/components/Table/TablePagination.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,16 @@ describe('Table Pagination', () => {
);
const rowsSelect = getByTestId('rowPerPageSelect');

fireEvent.change(rowsSelect, { target: { value: 20 }});
const appliedSelection = document.querySelector(
'select[data-testid=rowPerPageSelect]'
);

fireEvent.change(rowsSelect, { target: { value: 20 } });

expect(handlePageChange).toHaveBeenCalledWith(expect.any(Object), 1);
expect(handleRowsPerPageChange).toHaveBeenCalledWith('20');
expect(getByText(/1-20/i)).toBeInTheDocument();
expect(appliedSelection).toHaveDisplayValue('20');
});
});

Expand Down Expand Up @@ -195,9 +200,7 @@ describe('Table Pagination', () => {
});

it('should hide rows per page component when no onRowsPerPageChanged function passed', () => {
const { queryByText } = render(
<TablePagination itemCount={20}/>
);
const { queryByText } = render(<TablePagination itemCount={20} />);

expect(queryByText('Rows per page:')).not.toBeInTheDocument();
});
Expand Down

2 comments on commit a869e09

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀 Deployed on https://next--upbeat-sinoussi-f675aa.netlify.app

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀 Deployed on https://storybook-preview-dev--upbeat-sinoussi-f675aa.netlify.app

Please sign in to comment.