[GH-705] Auto-name new property (#1408)

* Open menu with property name and type when new property is added.

* Adding new property asks for the type and sets initial name.

* Component for property types menu items introduced.

* Close property menu when enter is pressed in the input.

* Unit test for `CardDetailProperties` fixed:
 - jest snapshot updated
 - switched from mocking `fetch` to mocking `mutator`

* Unit tests for `CardDetailProperties` updated:
 - use the recommended way to get elements and trigger user events
 - test for properties menu when adding new property added

* Unit tests for `CardDetailProperties` added:
 - delete existing property
 - add new property

* Don't use debouncing for property type selection.

* Fix unit test for deleting property.

* Width of button `+ Add a property` is fixed.

* Jest snapshot updated after merge with `main`
This commit is contained in:
kamre 2021-10-27 14:04:15 +07:00 committed by GitHub
parent c9a629d135
commit c5a4dd80f7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 599 additions and 144 deletions

View file

@ -287,13 +287,19 @@ exports[`components/cardDialog return cardDialog menu content 1`] = `
<div
class="octo-propertyname add-property"
>
<button
type="button"
<div
aria-label="menuwrapper"
class="MenuWrapper"
role="button"
>
<span>
+ Add a property
</span>
</button>
<button
type="button"
>
<span>
+ Add a property
</span>
</button>
</div>
</div>
</div>
<hr />
@ -693,13 +699,19 @@ exports[`components/cardDialog should match snapshot 1`] = `
<div
class="octo-propertyname add-property"
>
<button
type="button"
<div
aria-label="menuwrapper"
class="MenuWrapper"
role="button"
>
<span>
+ Add a property
</span>
</button>
<button
type="button"
>
<span>
+ Add a property
</span>
</button>
</div>
</div>
</div>
<hr />

View file

@ -45,14 +45,378 @@ exports[`components/cardDetail/CardDetailProperties should match snapshot 1`] =
<div
class="octo-propertyname add-property"
>
<button
class="Button"
type="button"
<div
aria-label="menuwrapper"
class="MenuWrapper"
role="button"
>
<span>
+ Add a property
</span>
</button>
<button
class="Button"
type="button"
>
<span>
+ Add a property
</span>
</button>
</div>
</div>
</div>
</div>
`;
exports[`components/cardDetail/CardDetailProperties should show property types menu 1`] = `
<div>
<div
class="octo-propertylist CardDetailProperties"
>
<div
class="octo-propertyrow"
>
<div
aria-label="menuwrapper"
class="MenuWrapper"
role="button"
>
<div
class="octo-propertyname"
>
<button
class="Button"
type="button"
>
<span>
Owner
</span>
</button>
</div>
</div>
<div
class="octo-propertyvalue"
data-testid="select-non-editable"
tabindex="0"
>
<span
class="Label propColorDefault "
>
<span
class="Label-text"
>
Jean-Luc Picard
</span>
</span>
</div>
</div>
<div
class="octo-propertyname add-property"
>
<div
aria-label="menuwrapper"
class="MenuWrapper"
role="button"
>
<button
class="Button"
type="button"
>
<span>
+ Add a property
</span>
</button>
<div
class="Menu noselect bottom"
>
<div
class="menu-contents"
>
<div
class="menu-options"
>
<div
class="MenuOption LabelOption menu-option"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
<b>
Select property type
</b>
</div>
<div
class="noicon"
/>
</div>
<div
class="MenuOption MenuSeparator menu-separator"
/>
<div
aria-label="Text"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Text
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Number"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Number
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Email"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Email
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Phone"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Phone
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="URL"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
URL
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Select"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Select
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Multi Select"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Multi Select
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Date"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Date
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Person"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Person
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Checkbox"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Checkbox
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Created time"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Created time
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Created by"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Created by
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Last updated time"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Last updated time
</div>
<div
class="noicon"
/>
</div>
<div
aria-label="Last updated by"
class="MenuOption TextOption menu-option"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Last updated by
</div>
<div
class="noicon"
/>
</div>
</div>
<div
class="menu-spacer hideOnWidescreen"
/>
<div
class="menu-options hideOnWidescreen"
>
<div
aria-label="Cancel"
class="MenuOption TextOption menu-option menu-cancel"
role="button"
>
<div
class="noicon"
/>
<div
class="menu-name"
>
Cancel
</div>
<div
class="noicon"
/>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>

View file

@ -26,11 +26,6 @@
display: flex;
flex-direction: column;
width: 100%;
.MenuWrapper {
position: relative;
align-self: center;
}
}
.octo-propertyrow {

View file

@ -2,23 +2,24 @@
// See LICENSE.txt for license information.
import React from 'react'
import {fireEvent, render} from '@testing-library/react'
import {render, screen, act} from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import '@testing-library/jest-dom'
import {mocked} from 'ts-jest/utils'
import {createIntl} from 'react-intl'
import {wrapIntl} from '../../testUtils'
import {TestBlockFactory} from '../../test/testBlockFactory'
import {FetchMock} from '../../test/fetchMock'
import mutator from '../../mutator'
import {propertyTypesList, typeDisplayName} from '../../widgets/propertyMenu'
import 'isomorphic-fetch'
import {PropertyType} from '../../blocks/board'
import CardDetailProperties from './cardDetailProperties'
global.fetch = FetchMock.fn
beforeEach(() => {
FetchMock.fn.mockReset()
})
jest.mock('../../mutator')
const mockedMutator = mocked(mutator, true)
describe('components/cardDetail/CardDetailProperties', () => {
const board = TestBlockFactory.createBoard()
@ -51,66 +52,109 @@ describe('components/cardDetail/CardDetailProperties', () => {
view.fields.sortOptions = []
view.fields.groupById = undefined
view.fields.hiddenOptionIds = []
const views = [view]
const card = TestBlockFactory.createCard(board)
card.fields.properties.property_id_1 = 'property_value_id_1'
const cards = [card]
const cardTemplate = TestBlockFactory.createCard(board)
cardTemplate.fields.isTemplate = true
const cardDetailProps = {
board,
card,
cards,
contents: [],
comments: [],
activeView: view,
views,
readonly: false,
}
test('should match snapshot', async () => {
const component = wrapIntl((
<CardDetailProperties
board={board!}
card={card}
cards={[card]}
contents={[]}
comments={[]}
activeView={view}
views={[view]}
readonly={false}
/>
))
const {container} = render(component)
it('should match snapshot', async () => {
const {container} = render(
wrapIntl(
<CardDetailProperties {...cardDetailProps}/>,
),
)
expect(container).toMatchSnapshot()
})
test('rename select property', async () => {
const component = wrapIntl((
<CardDetailProperties
board={board!}
card={card}
cards={[card]}
contents={[]}
comments={[]}
activeView={view}
views={[view]}
readonly={false}
/>
))
it('should rename existing select property', async () => {
render(
wrapIntl(
<CardDetailProperties {...cardDetailProps}/>,
),
)
const {container} = render(component)
const propertyLabel = container.querySelector('.MenuWrapper')
expect(propertyLabel).toBeDefined()
fireEvent.click(propertyLabel!)
const menuElement = screen.getByRole('button', {name: 'Owner'})
userEvent.click(menuElement)
const propertyNameInput = container.querySelector('.PropertyMenu.menu-textbox')
expect(propertyNameInput).toBeDefined()
userEvent.type(propertyNameInput!, 'Owner - Renamed{enter}')
fireEvent.click(propertyLabel!)
const newName = 'Owner - Renamed'
const propertyNameInput = screen.getByRole('textbox')
userEvent.type(propertyNameInput, `${newName}{enter}`)
// should be called once on renaming the property
expect(FetchMock.fn).toBeCalledTimes(1)
const propertyTemplate = board.fields.cardProperties[0]
expect(mockedMutator.changePropertyTypeAndName).toHaveBeenCalledTimes(1)
expect(mockedMutator.changePropertyTypeAndName).toHaveBeenCalledWith(board, cards, propertyTemplate, 'select', newName)
})
// Verify API call was made with renamed property
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const lastAPICallPayload = JSON.parse(FetchMock.fn.mock.calls[0][1].body)
expect(lastAPICallPayload[0].fields.cardProperties[0].name).toBe('Owner - Renamed')
expect(lastAPICallPayload[0].fields.cardProperties[0].options.length).toBe(3)
expect(lastAPICallPayload[0].fields.cardProperties[0].options[0].value).toBe('Jean-Luc Picard')
expect(lastAPICallPayload[0].fields.cardProperties[0].options[1].value).toBe('William Riker')
expect(lastAPICallPayload[0].fields.cardProperties[0].options[2].value).toBe('Deanna Troi')
it('should show confirmation dialog when deleting existing select property', () => {
render(
wrapIntl(
<CardDetailProperties {...cardDetailProps}/>,
),
)
const menuElement = screen.getByRole('button', {name: 'Owner'})
userEvent.click(menuElement)
const deleteButton = screen.getByRole('button', {name: /delete/i})
userEvent.click(deleteButton)
expect(screen.getByRole('heading', {name: 'Confirm Delete Property'})).toBeInTheDocument()
expect(screen.getByRole('button', {name: /delete/i})).toBeInTheDocument()
})
it('should show property types menu', () => {
const intl = createIntl({locale: 'en'})
const {container} = render(
wrapIntl(
<CardDetailProperties {...cardDetailProps}/>,
),
)
const menuElement = screen.getByRole('button', {name: /add a property/i})
userEvent.click(menuElement)
expect(container).toMatchSnapshot()
const selectProperty = screen.getByText(/select property type/i)
expect(selectProperty).toBeInTheDocument()
propertyTypesList.forEach((type: PropertyType) => {
const typeButton = screen.getByRole('button', {name: typeDisplayName(intl, type)})
expect(typeButton).toBeInTheDocument()
})
})
it('should add new number property', async () => {
render(
wrapIntl(
<CardDetailProperties {...cardDetailProps}/>,
),
)
const menuElement = screen.getByRole('button', {name: /add a property/i})
userEvent.click(menuElement)
await act(async () => {
const numberType = screen.getByRole('button', {name: /number/i})
userEvent.click(numberType)
})
expect(mockedMutator.insertPropertyTemplate).toHaveBeenCalledTimes(1)
const args = mockedMutator.insertPropertyTemplate.mock.calls[0]
const template = args[3]
expect(template).toBeTruthy()
expect(template!.name).toMatch(/number/i)
expect(template!.type).toBe('number')
})
})

View file

@ -1,10 +1,9 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React, {useEffect, useState} from 'react'
import {FormattedMessage, useIntl} from 'react-intl'
import React, {useState} from 'react'
import {useIntl, FormattedMessage} from 'react-intl'
import {Board, PropertyType, IPropertyTemplate} from '../../blocks/board'
import {Board, IPropertyTemplate, PropertyType} from '../../blocks/board'
import {Card} from '../../blocks/card'
import {BoardView} from '../../blocks/boardView'
import {ContentBlock} from '../../blocks/contentBlock'
@ -12,11 +11,13 @@ import {CommentBlock} from '../../blocks/commentBlock'
import mutator from '../../mutator'
import Button from '../../widgets/buttons/button'
import MenuWrapper from '../../widgets/menuWrapper'
import PropertyMenu from '../../widgets/propertyMenu'
import PropertyMenu, {PropertyTypes, typeDisplayName} from '../../widgets/propertyMenu'
import PropertyValueElement from '../propertyValueElement'
import {ConfirmationDialogBox} from '../confirmationDialogBox'
import {sendFlashMessage} from '../flashMessages'
import Menu from '../../widgets/menu'
import {IDType, Utils} from '../../utils'
type Props = {
board: Board
@ -30,8 +31,16 @@ type Props = {
}
const CardDetailProperties = React.memo((props: Props) => {
const intl = useIntl()
const {board, card, cards, views, activeView, contents, comments} = props
const [newTemplateId, setNewTemplateId] = useState('')
const intl = useIntl()
useEffect(() => {
const newProperty = board.fields.cardProperties.find((property) => property.id === newTemplateId)
if (newProperty) {
setNewTemplateId('')
}
}, [newTemplateId, board.fields.cardProperties])
const [showConfirmationDialog, setShowConfirmationDialog] = useState<boolean>(false)
const [deletingPropId, setDeletingPropId] = useState<string>('')
@ -48,7 +57,7 @@ const CardDetailProperties = React.memo((props: Props) => {
>
{props.readonly && <div className='octo-propertyname'>{propertyTemplate.name}</div>}
{!props.readonly &&
<MenuWrapper>
<MenuWrapper isOpen={propertyTemplate.id === newTemplateId}>
<div className='octo-propertyname'><Button>{propertyTemplate.name}</Button></div>
<PropertyMenu
propertyId={propertyTemplate.id}
@ -99,17 +108,29 @@ const CardDetailProperties = React.memo((props: Props) => {
{!props.readonly &&
<div className='octo-propertyname add-property'>
<Button
onClick={async () => {
// TODO: Show UI
await mutator.insertPropertyTemplate(board, activeView)
}}
>
<FormattedMessage
id='CardDetail.add-property'
defaultMessage='+ Add a property'
/>
</Button>
<MenuWrapper>
<Button>
<FormattedMessage
id='CardDetail.add-property'
defaultMessage='+ Add a property'
/>
</Button>
<Menu>
<PropertyTypes
label={intl.formatMessage({id: 'PropertyMenu.selectType', defaultMessage: 'Select property type'})}
onTypeSelected={async (type) => {
const template: IPropertyTemplate = {
id: Utils.createGuid(IDType.BlockID),
name: typeDisplayName(intl, type),
type,
options: [],
}
const templateId = await mutator.insertPropertyTemplate(board, activeView, -1, template)
setNewTemplateId(templateId)
}}
/>
</Menu>
</MenuWrapper>
</div>
}
</div>

View file

@ -224,10 +224,10 @@ class Mutator {
// Property Templates
async insertPropertyTemplate(board: Board, activeView: BoardView, index = -1, template?: IPropertyTemplate) {
async insertPropertyTemplate(board: Board, activeView: BoardView, index = -1, template?: IPropertyTemplate): Promise<string> {
if (!activeView) {
Utils.assertFailure('insertPropertyTemplate: no activeView')
return
return ''
}
const newTemplate = template || {
@ -257,6 +257,7 @@ class Mutator {
}
await this.updateBlocks(changedBlocks, oldBlocks, description)
return newTemplate.id
}
async duplicatePropertyTemplate(board: Board, activeView: BoardView, propertyId: string) {

View file

@ -10,11 +10,12 @@ type Props = {
stopPropagationOnToggle?: boolean;
className?: string
disabled?: boolean
isOpen?: boolean
}
const MenuWrapper = React.memo((props: Props) => {
const node = useRef<HTMLDivElement>(null)
const [open, setOpen] = useState(false)
const [open, setOpen] = useState(Boolean(props.isOpen))
if (!Array.isArray(props.children) || props.children.length !== 2) {
throw new Error('MenuWrapper needs exactly 2 children')

View file

@ -1,7 +1,6 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React, {useState, useRef, useEffect} from 'react'
import debounce from 'lodash/debounce'
import {useIntl, IntlShape} from 'react-intl'
import {PropertyType} from '../blocks/board'
@ -17,7 +16,7 @@ type Props = {
onDelete: (id: string) => void
}
function typeDisplayName(intl: IntlShape, type: PropertyType): string {
export function typeDisplayName(intl: IntlShape, type: PropertyType): string {
switch (type) {
case 'text': return intl.formatMessage({id: 'PropertyType.Text', defaultMessage: 'Text'})
case 'number': return intl.formatMessage({id: 'PropertyType.Number', defaultMessage: 'Number'})
@ -44,6 +43,52 @@ function typeMenuTitle(intl: IntlShape, type: PropertyType): string {
return `${intl.formatMessage({id: 'PropertyMenu.typeTitle', defaultMessage: 'Type'})}: ${typeDisplayName(intl, type)}`
}
export const propertyTypesList: PropertyType[] = [
'text',
'number',
'email',
'phone',
'url',
'select',
'multiSelect',
'date',
'person',
'checkbox',
'createdTime',
'createdBy',
'updatedTime',
'updatedBy',
]
type TypesProps = {
label: string
onTypeSelected: (type: PropertyType) => void
}
export const PropertyTypes = (props: TypesProps): JSX.Element => {
const intl = useIntl()
return (
<>
<Menu.Label>
<b>{props.label}</b>
</Menu.Label>
<Menu.Separator/>
{
propertyTypesList.map((type) => (
<Menu.Text
key={type}
id={type}
name={typeDisplayName(intl, type)}
onClick={() => props.onTypeSelected(type)}
/>
))
}
</>
)
}
const PropertyMenu = React.memo((props: Props) => {
const intl = useIntl()
const nameTextbox = useRef<HTMLInputElement>(null)
@ -54,30 +99,11 @@ const PropertyMenu = React.memo((props: Props) => {
defaultMessage: 'Delete',
})
const debouncedOnTypeAndNameChanged = (newType: PropertyType) => debounce(() => props.onTypeAndNameChanged(newType, name), 150)
useEffect(() => {
nameTextbox.current?.focus()
nameTextbox.current?.setSelectionRange(0, name.length)
}, [])
const propertyTypes = [
{type: 'text'},
{type: 'number'},
{type: 'email'},
{type: 'phone'},
{type: 'url'},
{type: 'select'},
{type: 'multiSelect'},
{type: 'date'},
{type: 'person'},
{type: 'checkbox'},
{type: 'createdTime'},
{type: 'createdBy'},
{type: 'updatedTime'},
{type: 'updatedBy'},
]
return (
<Menu>
<input
@ -85,13 +111,18 @@ const PropertyMenu = React.memo((props: Props) => {
type='text'
className='PropertyMenu menu-textbox'
onClick={(e) => e.stopPropagation()}
onChange={(e) => setName(e.target.value)}
onChange={(e) => {
setName(e.target.value)
}}
value={name}
onBlur={() => props.onTypeAndNameChanged(props.propertyType, name)}
onKeyDown={(e) => {
if (e.keyCode === 13 || e.keyCode === 27) {
if (e.key === 'Enter' || e.key === 'Escape') {
props.onTypeAndNameChanged(props.propertyType, name)
e.stopPropagation()
if (e.key === 'Enter') {
e.target.dispatchEvent(new Event('menuItemClicked'))
}
}
}}
spellCheck={true}
@ -100,24 +131,10 @@ const PropertyMenu = React.memo((props: Props) => {
id='type'
name={typeMenuTitle(intl, props.propertyType)}
>
<Menu.Label>
<b>
{intl.formatMessage({id: 'PropertyMenu.changeType', defaultMessage: 'Change property type'})}
</b>
</Menu.Label>
<Menu.Separator/>
{
propertyTypes.map((propertyType) => (
<Menu.Text
key={propertyType.type}
id={propertyType.type}
name={typeDisplayName(intl, propertyType.type as PropertyType)}
onClick={() => debouncedOnTypeAndNameChanged(propertyType.type as PropertyType)()}
/>
))
}
<PropertyTypes
label={intl.formatMessage({id: 'PropertyMenu.changeType', defaultMessage: 'Change property type'})}
onTypeSelected={(type: PropertyType) => props.onTypeAndNameChanged(type, name)}
/>
</Menu.SubMenu>
<Menu.Text
id='delete'