Skip to content

Commit

Permalink
Trim titles and values in basic settings (#9598)
Browse files Browse the repository at this point in the history
The `buildBasicSettingsDataset` call is used for:
- `block_formats`
- `font_family_formats`
- `font_size_formats`

The documented values for both `block_formats` and
`font_family_formats` include a space between the `;` and the title, for
example the default value in `block_formats` is:

```
Paragraph=p; Heading 1=h1; Heading 2=h2; Heading 3=h3; Heading 4=h4; Heading 5=h5; Heading 6=h6;
```

However the `split()` method in `SelectDatasets.ts` splits on the
delimiter, leading to values such as ` Heading 1=h1` which are then
passed into the `process()` method which just split them on `=` leading
to a title of ` Heading 1` (with leading whitespace) which does not
match any valid translation.

This patch modifies the `split()` method to trim the title component.

It also refactors the SelectDatasets module to move the configuration
processing into a utility method, and allows it to be unit tested.

Additionally any white-spare around the `=` or `;` leads to erroneous
values, for example a value of:

```
Paragraph = p ;
```

Leads to:
title: `Paragraph `
value: ` p `

And multiple contiguous spaces leads to additional values, for example:
```
12pt  14pt 16pt  18pt
```

Leads to a set of values:
- `12pt`
- `` (empty value)
- `14pt`
- `16pt`
- `18pt`
  • Loading branch information
andrewnicols committed Apr 30, 2024
1 parent c5ef59d commit da9061a
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 38 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/tinymce-GH-9599-2024-04-30.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
project: tinymce
kind: Fixed
body: Select Dataset content should be trimmed and filtered before parsing
time: 2024-04-30T11:26:37.663244+08:00
custom:
Issue: GH-9599
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { UiFinder } from '@ephox/agar';
import { describe, it } from '@ephox/bedrock-client';
import { Strings } from '@ephox/katamari';
import { SugarBody, TextContent } from '@ephox/sugar';
import { TinyHooks, TinySelections } from '@ephox/wrap-mcagar';
import { assert } from 'chai';
Expand All @@ -22,7 +21,7 @@ describe('browser.tinymce.core.FontSelectCustomTest', () => {

const assertSelectBoxDisplayValue = (title: string, expectedValue: string) => {
const selectBox = UiFinder.findIn(SugarBody.body(), `*[data-mce-name="${title}"]`).getOrDie();
const value = Strings.trim(TextContent.get(selectBox) ?? '');
const value = TextContent.get(selectBox) ?? '';
assert.equal(value, expectedValue, 'Should be the expected display value');
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import { Arr } from '@ephox/katamari';

import Editor from 'tinymce/core/api/Editor';

import { UiFactoryBackstageForStyleFormats } from '../../../backstage/StyleFormatsBackstage';

export interface BasicSelectItem {
readonly title: string;
readonly format: string;
readonly icon?: string;
}
import { BasicSelectItem, Delimiter, makeBasicSelectItems } from './utils/Select';

export interface BasicSelectDataset {
readonly type: 'basic';
Expand All @@ -21,47 +14,24 @@ export interface AdvancedSelectDataset extends UiFactoryBackstageForStyleFormats

export type SelectDataset = BasicSelectDataset | AdvancedSelectDataset;

const process = (rawFormats: string[]): BasicSelectItem[] => Arr.map(rawFormats, (item) => {
let title = item, format = item;
// Allow text=value block formats
const values = item.split('=');
if (values.length > 1) {
title = values[0];
format = values[1];
}

return { title, format };
});

const buildBasicStaticDataset = (data: Array<BasicSelectItem>): BasicSelectDataset => ({
type: 'basic',
data
});

export enum Delimiter {
SemiColon,
Space
}

const split = (rawFormats: string, delimiter: Delimiter): string[] => {
if (delimiter === Delimiter.SemiColon) {
return rawFormats.replace(/;$/, '').split(';');
} else {
return rawFormats.split(' ');
}
};

const buildBasicSettingsDataset = (editor: Editor, settingName: 'block_formats' | 'font_family_formats' | 'font_size_formats', delimiter: Delimiter): BasicSelectDataset => {
// eslint-disable-next-line @tinymce/no-direct-editor-options
const rawFormats = editor.options.get(settingName);
const data = process(split(rawFormats, delimiter));
const data = makeBasicSelectItems(rawFormats, delimiter);
return {
type: 'basic',
data
data,
};
};

export {
buildBasicSettingsDataset,
buildBasicStaticDataset
buildBasicStaticDataset,
Delimiter,
BasicSelectItem
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { Arr } from '@ephox/katamari';

import { Strings} from '@ephox/katamari';

export interface BasicSelectItem {
readonly title: string;
readonly format: string;
readonly icon?: string;
}

export enum Delimiter {
SemiColon = ';',
Space = ' '
}

const process = (rawFormats: string[]): BasicSelectItem[] => Arr.map(rawFormats, (item) => {
let title = item, format = item;
// Allow text=value block formats
const values = item.split('=').filter((value) => Strings.isNotEmpty(value));
if (values.length > 1) {
title = values[0];
format = values[1];
}

title = Strings.trim(title);
format = Strings.trim(format);

return { title, format };
});

const split = (rawFormats: string, delimiter: Delimiter): string[] => {
if (delimiter === Delimiter.SemiColon) {
return rawFormats.replace(/;$/, '').split(';');
} else {
return rawFormats.split(' ');
}
};

const makeBasicSelectItems = (rawFormats: string, delimiter: Delimiter): BasicSelectItem[] => process(split(rawFormats, delimiter));

export {
makeBasicSelectItems
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { context, describe, it } from '@ephox/bedrock-client';
import { assert } from 'chai';

import { Delimiter, makeBasicSelectItems } from 'tinymce/themes/silver/ui/core/complex/utils/Select';

describe('atomic.tinymce.themes.silver.ui.core.complex.utils.Select', () => {
context('makeBasicSelectItems', () => {
it('Works with a semicolon delimiter', () => {
const config = 'Paragraph=p; Heading 1=h1;Heading 2=h2; Heading 3 =h3;Pre =pre';
const items = makeBasicSelectItems(config, Delimiter.SemiColon);
assert.isArray(items);
assert.lengthOf(items, 5);
assert.deepEqual(items, [
{ title: 'Paragraph', format: 'p' },
{ title: 'Heading 1', format: 'h1' },
{ title: 'Heading 2', format: 'h2' },
{ title: 'Heading 3', format: 'h3' },
{ title: 'Pre', format: 'pre' }
]);
});
it('Works with a space delimiter', () => {
const config = '8pt 10pt 12pt 14pt 18pt 24pt 36pt';
const items = makeBasicSelectItems(config, Delimiter.Space);
assert.isArray(items);
assert.lengthOf(items, 7);
assert.deepEqual(items, [
{ title: '8pt', format: '8pt' },
{ title: '10pt', format: '10pt' },
{ title: '12pt', format: '12pt' },
{ title: '14pt', format: '14pt' },
{ title: '18pt', format: '18pt' },
{ title: '24pt', format: '24pt' },
{ title: '36pt', format: '36pt' }
]);
});
it('Works with a mix of text/value items', () => {
const config = 'Paragraph = p ; pre';
const items = makeBasicSelectItems(config, Delimiter.SemiColon);
assert.isArray(items);
assert.lengthOf(items, 2);
assert.deepEqual(items, [
{ title: 'Paragraph', format: 'p' },
{ title: 'pre', format: 'pre' }
]);
});
});
});

0 comments on commit da9061a

Please sign in to comment.