From df2313e1819d48ec3bc4a8f03ac0eabc7391fee6 Mon Sep 17 00:00:00 2001 From: Julie Plantey Date: Thu, 22 Dec 2022 12:35:40 +0100 Subject: [PATCH 1/3] improve read-only state --- .../components/RelationInput/RelationInput.js | 12 +++++------- .../RelationInput/components/RelationItem.js | 3 ++- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js b/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js index 85e9ece250..4bfcb77071 100644 --- a/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js +++ b/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js @@ -38,12 +38,13 @@ export const LinkEllipsis = styled(Link)` export const DisconnectButton = styled.button` svg path { - fill: ${({ theme }) => theme.colors.neutral500}; + fill: ${({ theme, disabled }) => + !disabled ? theme.colors.neutral500 : theme.colors.neutral600}; } &:hover svg path, &:focus svg path { - fill: ${({ theme }) => theme.colors.neutral600}; + fill: ${({ theme, disabled }) => !disabled && theme.colors.neutral600}; } `; @@ -103,8 +104,7 @@ const RelationInput = ({ ); const shouldDisplayLoadMoreButton = - (!!labelLoadMore && !disabled && paginatedRelations.hasNextPage) || - paginatedRelations.isLoading; + (!!labelLoadMore && paginatedRelations.hasNextPage) || paginatedRelations.isLoading; const options = useMemo( () => @@ -503,9 +503,7 @@ const ListItem = ({ data, index, style }) => { {href ? ( - - {mainField ?? id} - + {mainField ?? id} ) : ( {mainField ?? id} diff --git a/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js b/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js index 64fde5e50e..48b3fc8b4b 100644 --- a/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js +++ b/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js @@ -83,7 +83,7 @@ export const RelationItem = ({ paddingRight={4} hasRadius borderSize={1} - background={disabled ? 'neutral150' : 'neutral0'} + background="neutral0" borderColor="neutral200" justifyContent="space-between" ref={canDrag ? composedRefs : undefined} @@ -99,6 +99,7 @@ export const RelationItem = ({ aria-label={iconButtonAriaLabel} noBorder onKeyDown={handleKeyDown} + disabled={disabled} > From 6d49a3c40b6c1f94d1a3425674d15860f2b4d555 Mon Sep 17 00:00:00 2001 From: Julie Plantey Date: Fri, 23 Dec 2022 10:43:23 +0100 Subject: [PATCH 2/3] updated tests --- .../RelationInput/tests/RelationInput.test.js | 23 +++++++++++++------ .../__snapshots__/RelationInput.test.js.snap | 2 +- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/RelationInput.test.js b/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/RelationInput.test.js index 1ed85e5793..45050847f4 100644 --- a/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/RelationInput.test.js +++ b/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/RelationInput.test.js @@ -136,12 +136,12 @@ describe('Content-Manager || RelationInput', () => { }); test('should call onRelationLoadMore', () => { - const spy = jest.fn(); - setup({ onRelationLoadMore: spy }); + const onRelationLoadMoreSpy = jest.fn(); + setup({ onRelationLoadMore: onRelationLoadMoreSpy }); fireEvent.click(screen.getByText('Load more')); - expect(spy).toHaveBeenCalled(); + expect(onRelationLoadMoreSpy).toHaveBeenCalled(); }); test('should call onSearch', () => { @@ -306,7 +306,7 @@ describe('Content-Manager || RelationInput', () => { }, }); - expect(screen.getByRole('button', { name: /load more/i })).toHaveAttribute( + expect(screen.getByRole('button', { name: 'Load more' })).toHaveAttribute( 'aria-disabled', 'true' ); @@ -318,12 +318,21 @@ describe('Content-Manager || RelationInput', () => { expect(screen.getByText('This is an error')).toBeInTheDocument(); }); - test('should apply disabled state', () => { - const { queryByText, getByTestId, container } = setup({ disabled: true }); + test('should display disabled state with only read permission', () => { + const onRelationLoadMoreSpy = jest.fn(); + const { getAllByRole, getByRole, getByTestId, getByText, container } = setup({ + disabled: true, + onRelationLoadMore: onRelationLoadMoreSpy, + }); + + fireEvent.click(getByText('Load more')); + expect(onRelationLoadMoreSpy).toHaveBeenCalledTimes(1); - expect(queryByText('Load more')).not.toBeInTheDocument(); expect(container.querySelector('input')).toBeDisabled(); expect(getByTestId('remove-relation-1')).toBeDisabled(); + const [dragButton] = getAllByRole('button', { name: 'Drag' }); + expect(dragButton).toHaveAttribute('aria-disabled', 'true'); + expect(getByRole('link', { name: 'Relation 1' })).toBeInTheDocument(); }); }); }); diff --git a/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/__snapshots__/RelationInput.test.js.snap b/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/__snapshots__/RelationInput.test.js.snap index 2976075dd7..4c743665e7 100644 --- a/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/__snapshots__/RelationInput.test.js.snap +++ b/packages/core/admin/admin/src/content-manager/components/RelationInput/tests/__snapshots__/RelationInput.test.js.snap @@ -617,7 +617,7 @@ exports[`Content-Manager || RelationInput should render and match snapshot 1`] = Select...
Date: Tue, 27 Dec 2022 10:33:34 +0100 Subject: [PATCH 3/3] UI feedback fix --- .../content-manager/components/RelationInput/RelationInput.js | 2 +- .../components/RelationInput/components/RelationItem.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js b/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js index 4bfcb77071..154b669329 100644 --- a/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js +++ b/packages/core/admin/admin/src/content-manager/components/RelationInput/RelationInput.js @@ -39,7 +39,7 @@ export const LinkEllipsis = styled(Link)` export const DisconnectButton = styled.button` svg path { fill: ${({ theme, disabled }) => - !disabled ? theme.colors.neutral500 : theme.colors.neutral600}; + disabled ? theme.colors.neutral600 : theme.colors.neutral500}; } &:hover svg path, diff --git a/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js b/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js index 48b3fc8b4b..e415174d0b 100644 --- a/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js +++ b/packages/core/admin/admin/src/content-manager/components/RelationInput/components/RelationItem.js @@ -83,8 +83,8 @@ export const RelationItem = ({ paddingRight={4} hasRadius borderSize={1} - background="neutral0" borderColor="neutral200" + background={disabled ? 'neutral150' : 'neutral0'} justifyContent="space-between" ref={canDrag ? composedRefs : undefined} data-handler-id={handlerId}