[Workplace Search] Make Inline Editable Table slightly more accessible (#122193)

* [Workplace Search] Make Inline Editable Table slightly more accessible

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
sphilipse 2022-01-10 19:43:58 +01:00 committed by GitHub
parent a9b82b0740
commit 1ca5b89038
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 105 additions and 54 deletions

View file

@ -1,8 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
export const EMPTY_ITEM = { id: null };

View file

@ -88,14 +88,38 @@ describe('InlineEditableTable', () => {
const reorderableTable = wrapper.find(ReorderableTable);
expect(reorderableTable.exists()).toBe(true);
expect(reorderableTable.prop('items')).toEqual(items);
expect(wrapper.find('[data-test-subj="actionButton"]').children().text()).toEqual('New row');
expect(
wrapper.find('[data-test-subj="inlineEditableTableActionButton"]').children().text()
).toEqual('New row');
});
it('renders a title if one is provided', () => {
const wrapper = shallow(
<InlineEditableTableContents {...requiredParams} description={<p>Some Description</p>} />
);
expect(wrapper.find('[data-test-subj="inlineEditableTableTitle"]').exists()).toBe(true);
});
it('does not render a title if none is provided', () => {
const wrapper = shallow(
<InlineEditableTableContents
{...{ ...requiredParams, title: '' }}
description={<p>Some Description</p>}
/>
);
expect(wrapper.find('[data-test-subj="inlineEditableTableTitle"]').exists()).toBe(false);
});
it('renders a description if one is provided', () => {
const wrapper = shallow(
<InlineEditableTableContents {...requiredParams} description={<p>Some Description</p>} />
);
expect(wrapper.find('[data-test-subj="description"]').exists()).toBe(true);
expect(wrapper.find('[data-test-subj="inlineEditableTableDescription"]').exists()).toBe(true);
});
it('renders no description if none is provided', () => {
const wrapper = shallow(<InlineEditableTableContents {...requiredParams} />);
expect(wrapper.find('[data-test-subj="inlineEditableTableDescription"]').exists()).toBe(false);
});
it('can specify items in the table that are uneditable', () => {
@ -117,9 +141,9 @@ describe('InlineEditableTable', () => {
const wrapper = shallow(
<InlineEditableTableContents {...requiredParams} addButtonText="Add a new row custom text" />
);
expect(wrapper.find('[data-test-subj="actionButton"]').children().text()).toEqual(
'Add a new row custom text'
);
expect(
wrapper.find('[data-test-subj="inlineEditableTableActionButton"]').children().text()
).toEqual('Add a new row custom text');
});
describe('when a user is editing an unsaved item', () => {

View file

@ -18,7 +18,6 @@ import { ReorderableTable } from '../reorderable_table';
import { ItemWithAnID } from '../types';
import { EMPTY_ITEM } from './constants';
import { getUpdatedColumns } from './get_updated_columns';
import { InlineEditableTableLogic } from './inline_editable_table_logic';
import { FormErrors, InlineEditableTableColumn } from './types';
@ -103,16 +102,16 @@ export const InlineEditableTableContents = <Item extends ItemWithAnID>({
const isEditingItem = (item: Item) => item.id === editingItemId;
const isActivelyEditing = (item: Item) => isEditing && isEditingItem(item);
const emptyItem = { id: null } as Item;
const displayedItems = isEditingUnsavedItem
? uneditableItems
? [EMPTY_ITEM, ...items]
: [...items, EMPTY_ITEM]
? [emptyItem, ...items]
: [...items, emptyItem]
: items;
const updatedColumns = getUpdatedColumns({
columns,
// TODO We shouldn't need this cast here
displayedItems: displayedItems as Item[],
displayedItems,
isActivelyEditing,
canRemoveLastItem,
isLoading,
@ -124,14 +123,16 @@ export const InlineEditableTableContents = <Item extends ItemWithAnID>({
<>
<EuiFlexGroup alignItems="center">
<EuiFlexItem>
<EuiTitle size="xs">
<h3>{title}</h3>
</EuiTitle>
{!!title && (
<EuiTitle size="xs" data-test-subj="inlineEditableTableTitle">
<h3>{title}</h3>
</EuiTitle>
)}
{!!description && (
<>
<EuiSpacer size="s" />
<EuiText
data-test-subj="description"
data-test-subj="inlineEditableTableDescription"
color="subdued"
size="s"
className="inlineEditableTable__descriptionText"
@ -148,7 +149,7 @@ export const InlineEditableTableContents = <Item extends ItemWithAnID>({
disabled={isEditing}
onClick={editNewItem}
color="success"
data-test-subj="actionButton"
data-test-subj="inlineEditableTableActionButton"
>
{addButtonText ||
i18n.translate('xpack.enterpriseSearch.inlineEditableTable.newRowButtonLabel', {
@ -160,8 +161,7 @@ export const InlineEditableTableContents = <Item extends ItemWithAnID>({
<EuiSpacer size="m" />
<ReorderableTable
className={classNames(className, 'editableTable')}
// TODO don't cast
items={displayedItems as Item[]}
items={displayedItems}
unreorderableItems={uneditableItems}
columns={updatedColumns}
rowProps={(item) => ({

View file

@ -27,14 +27,14 @@ describe('InlineEditableTableLogic', () => {
const { mount } = new LogicMounter(InlineEditableTableLogic);
const DEFAULT_VALUES = {
editingItemId: null,
editingItemValue: null,
fieldErrors: {},
isEditing: false,
rowErrors: [],
};
const SELECTORS = {
editingItemId: null,
isEditing: false,
doesEditingItemValueContainEmptyProperty: false,
isEditingUnsavedItem: false,
};
@ -102,8 +102,6 @@ describe('InlineEditableTableLogic', () => {
describe('editNewItem', () => {
it('updates state to reflect a new item being edited', () => {
const logic = mountLogic({
isEditing: false,
editingItemId: 1,
editingItemValue: {
id: 1,
foo: 'some foo',
@ -113,8 +111,6 @@ describe('InlineEditableTableLogic', () => {
logic.actions.editNewItem();
expect(logicValuesWithoutSelectors(logic)).toEqual({
...DEFAULT_VALUES,
isEditing: true,
editingItemId: null,
editingItemValue: {
// Note that new values do not yet have an id
foo: '',
@ -127,8 +123,6 @@ describe('InlineEditableTableLogic', () => {
describe('editExistingItem', () => {
it('updates state to reflect the item that was passed being edited', () => {
const logic = mountLogic({
isEditing: false,
editingItemId: 1,
editingItemValue: {
id: 1,
foo: '',
@ -142,8 +136,6 @@ describe('InlineEditableTableLogic', () => {
});
expect(logicValuesWithoutSelectors(logic)).toEqual({
...DEFAULT_VALUES,
isEditing: true,
editingItemId: 2,
editingItemValue: {
id: 2,
foo: 'existing foo',
@ -208,11 +200,66 @@ describe('InlineEditableTableLogic', () => {
});
describe('selectors', () => {
describe('isEditing', () => {
it('is true when the user is currently editing an item', () => {
const logic = mountLogic({
editingItemValue: {
id: null,
foo: '',
},
});
expect(logic.values.isEditing).toBe(true);
});
it('is false when the user is NOT currently editing an item', () => {
const logic = mountLogic({
editingItemValue: null,
});
expect(logic.values.isEditing).toBe(false);
});
});
describe('editingItemId', () => {
it('equals the id of the currently edited item', () => {
const logic = mountLogic({
editingItemValue: {
id: 1,
foo: '',
},
});
expect(logic.values.editingItemId).toBe(1);
});
it('equals null if the currently edited item is a new unsaved item', () => {
const logic = mountLogic({
editingItemValue: {
id: null,
foo: '',
},
});
expect(logic.values.editingItemId).toBe(null);
});
it('is null when the user is NOT currently editing an item', () => {
const logic = mountLogic({
editingItemValue: null,
});
expect(logic.values.editingItemId).toBe(null);
});
});
describe('isEditingUnsavedItem', () => {
it('is true when the user is currently editing an unsaved item', () => {
const logic = mountLogic({
isEditing: true,
editingItemId: null,
editingItemValue: {
id: null,
foo: '',
},
});
expect(logic.values.isEditingUnsavedItem).toBe(true);

View file

@ -35,7 +35,6 @@ const getUnsavedItemId = () => null;
const doesIdMatchUnsavedId = (idToCheck: number) => idToCheck === getUnsavedItemId();
interface InlineEditableTableValues<Item extends ItemWithAnID> {
// TODO This could likely be a selector
isEditing: boolean;
// TODO we should editingItemValue have editingItemValue and editingItemId should be a selector
editingItemId: Item['id'] | null; // editingItem is null when the user is editing a new but not saved item
@ -81,22 +80,6 @@ export const InlineEditableTableLogic = kea<InlineEditableTableLogicType<ItemWit
setRowErrors: (rowErrors) => ({ rowErrors }),
}),
reducers: ({ props: { columns } }) => ({
isEditing: [
false,
{
doneEditing: () => false,
editNewItem: () => true,
editExistingItem: () => true,
},
],
editingItemId: [
null,
{
doneEditing: () => null,
editNewItem: () => getUnsavedItemId(),
editExistingItem: (_, { item }) => item.id,
},
],
editingItemValue: [
null,
{
@ -124,6 +107,11 @@ export const InlineEditableTableLogic = kea<InlineEditableTableLogicType<ItemWit
],
}),
selectors: ({ selectors }) => ({
editingItemId: [
() => [selectors.editingItemValue],
(editingItemValue) => editingItemValue?.id ?? null,
],
isEditing: [() => [selectors.editingItemValue], (editingItemValue) => !!editingItemValue],
isEditingUnsavedItem: [
() => [selectors.isEditing, selectors.editingItemId],
(isEditing, editingItemId) => {