Skip to content

Commit

Permalink
#19562 fix disabled on dot-key-values allowing to drag and… (#1942)
Browse files Browse the repository at this point in the history
* #19562 fix disabled on dot-key-values allowing to drag and drop

* added test to disabled drag and drop

* added error msg when duplicated key is trying to be added
  • Loading branch information
alfredo-dotcms authored Apr 12, 2022
1 parent 5d1dc71 commit a87d409
Show file tree
Hide file tree
Showing 7 changed files with 47,813 additions and 515 deletions.
24 changes: 16 additions & 8 deletions libs/dotcms-webcomponents/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,14 @@ export namespace Components {
* (optional) Disables field's interaction
*/
"disabled": boolean;
/**
* (optional) Text that will be shown when required is set and condition is not met
*/
"duplicatedKeyMessage": string;
/**
* (optional) The string containing the value to be parsed for whitelist key/value
*/
"errorExistingKey": boolean;
/**
* (optional) Label for the add button in the key-value-form
*/
Expand Down Expand Up @@ -632,10 +640,6 @@ export namespace Components {
* Value of the field
*/
"value": string;
/**
* (optional) The string containing the value to be parsed for whitelist key/value
*/
"whiteList": string;
/**
* (optional) The string to use in the empty option of whitelist dropdown key/value item
*/
Expand Down Expand Up @@ -1935,6 +1939,14 @@ declare namespace LocalJSX {
* (optional) Disables field's interaction
*/
"disabled"?: boolean;
/**
* (optional) Text that will be shown when required is set and condition is not met
*/
"duplicatedKeyMessage"?: string;
/**
* (optional) The string containing the value to be parsed for whitelist key/value
*/
"errorExistingKey"?: boolean;
/**
* (optional) Label for the add button in the key-value-form
*/
Expand Down Expand Up @@ -1989,10 +2001,6 @@ declare namespace LocalJSX {
* Value of the field
*/
"value"?: string;
/**
* (optional) The string containing the value to be parsed for whitelist key/value
*/
"whiteList"?: string;
/**
* (optional) The string to use in the empty option of whitelist dropdown key/value item
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('key-value-table', () => {

const rows = await element.findAll('tr');
expect(rows.length).toBe(2);
expect(rows[0].getAttribute('draggable')).toBeDefined();
expect(rows[0]).toEqualHtml(`
<tr>
<td>
Expand Down Expand Up @@ -55,6 +56,18 @@ describe('key-value-table', () => {
`);
});

it('should disable dragabble on rows', async () => {
element.setProperty('items', [
{ key: 'keyA', value: '1' },
{ key: 'keyB', value: '2' }
]);
element.setProperty('disabled', true);
await page.waitForChanges();

const rows = await element.findAll('tr');
expect(rows[0].getAttribute('draggable')).not.toBeDefined();
});

it('should handle invalid items', async () => {
element.setProperty('items', {
a: { key: 'keyA', value: '1' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,26 @@ export class KeyValueTableComponent {
// D&D - BEGIN

private bindDraggableEvents() {
const rows = document.querySelectorAll('key-value-table tr');
rows.forEach((row) => {
row.setAttribute('draggable', 'true');

row.removeEventListener('dragstart', this.handleDragStart.bind(this), false);
row.removeEventListener('dragenter', this.handleDragEnter, false);
row.removeEventListener('dragover', this.handleDragOver.bind(this), false);
row.removeEventListener('dragleave', this.handleDragLeave, false);
row.removeEventListener('drop', this.handleDrop.bind(this), false);
row.removeEventListener('dragend', this.handleDragEnd.bind(this), false);

row.addEventListener('dragstart', this.handleDragStart.bind(this), false);
row.addEventListener('dragenter', this.handleDragEnter, false);
row.addEventListener('dragover', this.handleDragOver.bind(this), false);
row.addEventListener('dragleave', this.handleDragLeave, false);
row.addEventListener('drop', this.handleDrop.bind(this), false);
row.addEventListener('dragend', this.handleDragEnd.bind(this), false);
});
if (!this.disabled) {
const rows = document.querySelectorAll('key-value-table tr');
rows.forEach((row) => {
row.setAttribute('draggable', 'true');

row.removeEventListener('dragstart', this.handleDragStart.bind(this), false);
row.removeEventListener('dragenter', this.handleDragEnter, false);
row.removeEventListener('dragover', this.handleDragOver.bind(this), false);
row.removeEventListener('dragleave', this.handleDragLeave, false);
row.removeEventListener('drop', this.handleDrop.bind(this), false);
row.removeEventListener('dragend', this.handleDragEnd.bind(this), false);

row.addEventListener('dragstart', this.handleDragStart.bind(this), false);
row.addEventListener('dragenter', this.handleDragEnter, false);
row.addEventListener('dragover', this.handleDragOver.bind(this), false);
row.addEventListener('dragleave', this.handleDragLeave, false);
row.addEventListener('drop', this.handleDrop.bind(this), false);
row.addEventListener('dragend', this.handleDragEnd.bind(this), false);
});
}
}

private removeElementById(elemId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,33 @@ describe('dot-key-value', () => {
});
});

describe('duplicatedKeyMessage', () => {
it('should show default', async () => {
element.setProperty('value', 'hello|world,hola|mundo');
await page.waitForChanges();

const form = await getForm();
form.triggerEvent('add', {
detail: {
key: 'hello',
value: 'hello dupped'
}
});
await page.waitForChanges();

const error = await dotTestUtil.getErrorMessage(page);
expect(error.textContent).toBe('The key already exist');
});

it('should not show', async () => {
element.setProperty('value', 'key|value');
await page.waitForChanges();

const error = await dotTestUtil.getErrorMessage(page);
expect(error).toBeNull();
});
});

describe('value', () => {
it('should set items', async () => {
element.setProperty('value', 'hello|world,hola|mundo');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ export class DotKeyValueComponent {
})
requiredMessage = 'This field is required';

/** (optional) Text that will be shown when required is set and condition is not met */
@Prop({
reflect: true
})
duplicatedKeyMessage = 'The key already exist';

/** (optional) Disables field's interaction */
@Prop({
reflect: true
Expand Down Expand Up @@ -137,6 +143,7 @@ export class DotKeyValueComponent {
@Prop({
reflect: true
})
errorExistingKey: boolean;
whiteList: string;

@State()
Expand Down Expand Up @@ -173,6 +180,7 @@ export class DotKeyValueComponent {
this.items = this.items.filter(
(_item: DotKeyValueField, index: number) => index !== event.detail
);
this.errorExistingKey = false;
this.refreshStatus();
this.emitChanges();
}
Expand Down Expand Up @@ -206,11 +214,14 @@ export class DotKeyValueComponent {

@Listen('add')
addItemHandler({ detail }: CustomEvent<DotKeyValueField>): void {
const itemExists = this.items.some((item: DotKeyValueField) => item.key === detail.key);
this.refreshStatus();

if ((this.uniqueKeys && !itemExists) || !this.uniqueKeys) {
this.errorExistingKey = this.items.some(
(item: DotKeyValueField) => item.key === detail.key
);

if ((this.uniqueKeys && !this.errorExistingKey) || !this.uniqueKeys) {
this.items = [...this.items, detail];
this.refreshStatus();
this.emitChanges();
}
}
Expand All @@ -222,7 +233,11 @@ export class DotKeyValueComponent {
}

render() {
const classes = getClassNames(this.status, this.isValid(), this.required);
const classes = getClassNames(
this.status,
this.isValid() && !this.errorExistingKey,
this.required
);

return (
<Host class={{ ...classes }}>
Expand Down Expand Up @@ -289,7 +304,15 @@ export class DotKeyValueComponent {
}

private getErrorMessage(): string {
return this.isValid() ? '' : this.requiredMessage;
let errorMsg = '';

if (this.errorExistingKey) {
errorMsg = this.duplicatedKeyMessage;
} else if (!this.isValid()) {
this.requiredMessage;
}

return errorMsg;
}

private refreshStatus(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
| Property | Attribute | Description | Type | Default |
| --------------------------- | ------------------------------- | ------------------------------------------------------------------------------------- | --------- | -------------------------- |
| `disabled` | `disabled` | (optional) Disables field's interaction | `boolean` | `false` |
| `duplicatedKeyMessage` | `duplicated-key-message` | (optional) Text that will be shown when required is set and condition is not met | `string` | `'The key already exist'` |
| `errorExistingKey` | `error-existing-key` | (optional) The string containing the value to be parsed for whitelist key/value | `boolean` | `undefined` |
| `formAddButtonLabel` | `form-add-button-label` | (optional) Label for the add button in the key-value-form | `string` | `undefined` |
| `formKeyLabel` | `form-key-label` | (optional) The string to use in the key label in the key-value-form | `string` | `undefined` |
| `formKeyPlaceholder` | `form-key-placeholder` | (optional) Placeholder for the key input text in the key-value-form | `string` | `undefined` |
Expand All @@ -23,7 +25,6 @@
| `requiredMessage` | `required-message` | (optional) Text that will be shown when required is set and condition is not met | `string` | `'This field is required'` |
| `uniqueKeys` | `unique-keys` | (optional) Allows unique keys only | `boolean` | `false` |
| `value` | `value` | Value of the field | `string` | `''` |
| `whiteList` | `white-list` | (optional) The string containing the value to be parsed for whitelist key/value | `string` | `undefined` |
| `whiteListEmptyOptionLabel` | `white-list-empty-option-label` | (optional) The string to use in the empty option of whitelist dropdown key/value item | `string` | `undefined` |


Expand Down
Loading

0 comments on commit a87d409

Please sign in to comment.