Skip to content

Commit

Permalink
Rich Text: Set applied format as active in applyFormats (#15573)
Browse files Browse the repository at this point in the history
* Rich Text: Set applied format as active in applyFormats

* Rich Text: Update toggleFormat tests per activeFormats revision

* Rich Text: Replace existing active format in applied

* Rich Text: Remove active format in all cases for removeFormat

* Rich Text: Update toggleFormat tests per removeFormats revision

* Testing: Update E2E test case to verify link display regression

* Rich Text: Verify by test of activeFormats format replacement
  • Loading branch information
aduth authored May 13, 2019
1 parent 6171097 commit 449c254
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 16 deletions.
6 changes: 6 additions & 0 deletions packages/e2e-tests/specs/__snapshots__/links.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,9 @@ exports[`Links should contain a label when it should open in a new tab 1`] = `
<p>This is <a href=\\"http://w.org\\" target=\\"_blank\\" rel=\\"noreferrer noopener\\" aria-label=\\"WordPress (opens in a new tab)\\">WordPress</a></p>
<!-- /wp:paragraph -->"
`;
exports[`Links should contain a label when it should open in a new tab 2`] = `
"<!-- wp:paragraph -->
<p>This is <a href=\\"http://wordpress.org\\">WordPress</a></p>
<!-- /wp:paragraph -->"
`;
43 changes: 43 additions & 0 deletions packages/e2e-tests/specs/links.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,5 +503,48 @@ describe( 'Links', () => {
await page.keyboard.press( 'Enter' );

expect( await getEditedPostContent() ).toMatchSnapshot();

// Regression Test: This verifies that the UI is updated according to
// the expected changed values, where previously the value could have
// fallen out of sync with how the UI is displayed (specifically for
// collapsed selections).
//
// See: https://github.com/WordPress/gutenberg/pull/15573

// Collapse selection.
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'ArrowRight' );
// Edit link.
await pressKeyWithModifier( 'primary', 'k' );
await waitForAutoFocus();
await pressKeyWithModifier( 'primary', 'a' );
await page.keyboard.type( 'wordpress.org' );
// Navigate to the settings toggle.
await page.keyboard.press( 'Tab' );
await page.keyboard.press( 'Tab' );
// Open settings.
await page.keyboard.press( 'Space' );
// Navigate to the "Open in New Tab" checkbox.
await page.keyboard.press( 'Tab' );
// Uncheck the checkbox.
await page.keyboard.press( 'Space' );
// Navigate back to the input field.
await page.keyboard.press( 'Tab' );
// Submit the form.
await page.keyboard.press( 'Enter' );

// Navigate back to inputs to verify appears as changed.
await pressKeyWithModifier( 'primary', 'k' );
await waitForAutoFocus();
const link = await page.evaluate( () => document.activeElement.value );
expect( link ).toBe( 'http://wordpress.org' );
await page.keyboard.press( 'Tab' );
await page.keyboard.press( 'Tab' );
await page.keyboard.press( 'Space' );
await page.keyboard.press( 'Tab' );
const isChecked = await page.evaluate( () => document.activeElement.checked );
expect( isChecked ).toBe( false );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );
23 changes: 13 additions & 10 deletions packages/rich-text/src/apply-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/

import { find } from 'lodash';
import { find, reject } from 'lodash';

/**
* Internal dependencies
Expand Down Expand Up @@ -34,7 +34,7 @@ export function applyFormat(
startIndex = value.start,
endIndex = value.end
) {
const { formats, activeFormats = [] } = value;
const { formats, activeFormats } = value;
const newFormats = formats.slice();

// The selection is collapsed.
Expand All @@ -59,13 +59,6 @@ export function applyFormat(
replace( newFormats[ endIndex ], index, format );
endIndex++;
}
// Otherwise, insert a placeholder with the format so new input appears
// with the format applied.
} else {
return {
...value,
activeFormats: [ ...activeFormats, format ],
};
}
} else {
// Determine the highest position the new format can be inserted at.
Expand All @@ -92,5 +85,15 @@ export function applyFormat(
}
}

return normaliseFormats( { ...value, formats: newFormats } );
return normaliseFormats( {
...value,
formats: newFormats,
// Always revise active formats. This serves as a placeholder for new
// inputs with the format so new input appears with the format applied,
// and ensures a format of the same type uses the latest values.
activeFormats: [
...reject( activeFormats, { type: format.type } ),
format,
],
} );
}
11 changes: 5 additions & 6 deletions packages/rich-text/src/remove-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ export function removeFormat(
filterFormats( newFormats, endIndex, formatType );
endIndex++;
}
} else {
return {
...value,
activeFormats: reject( activeFormats, { type: formatType } ),
};
}
} else {
for ( let i = startIndex; i < endIndex; i++ ) {
Expand All @@ -62,7 +57,11 @@ export function removeFormat(
}
}

return normaliseFormats( { ...value, formats: newFormats } );
return normaliseFormats( {
...value,
formats: newFormats,
activeFormats: reject( activeFormats, { type: formatType } ),
} );
}

function filterFormats( formats, index, formatType ) {
Expand Down
11 changes: 11 additions & 0 deletions packages/rich-text/src/test/apply-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ em ],
formats: [ [ em ], [ em ], [ em ], [ em ] ],
};
const result = applyFormat( deepFreeze( record ), em, 0, 4 );
Expand All @@ -39,6 +40,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ em ],
formats: [ [ strong, em ], [ strong, em ], [ strong, em ], [ strong, em ] ],
};
const result = applyFormat( deepFreeze( record ), em, 0, 4 );
Expand All @@ -55,6 +57,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ em ],
formats: [ [ strong, em ], [ strong, em ], [ strong, em ], [ strong, em ] ],
};
const result = applyFormat( deepFreeze( record ), em, 0, 4 );
Expand All @@ -71,6 +74,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ strong ],
formats: [ [ strong ], [ strong, em ], [ strong, em ], [ strong ] ],
};
const result = applyFormat( deepFreeze( record ), strong, 0, 4 );
Expand All @@ -87,6 +91,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ strong ],
formats: [ [ strong ], [ strong, em ], [ strong, em ], , ],
};
const result = applyFormat( deepFreeze( record ), strong, 0, 3 );
Expand All @@ -103,6 +108,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ strong ],
formats: [ , [ strong, em ], [ strong, em ], [ strong ] ],
};
const result = applyFormat( deepFreeze( record ), strong, 1, 4 );
Expand All @@ -119,6 +125,7 @@ describe( 'applyFormat', () => {
};
const expected = {
...record,
activeFormats: [ strong ],
formats: [ , [ strong, em ], [ strong ], [ strong, em ] ],
};
const result = applyFormat( deepFreeze( record ), strong, 1, 4 );
Expand All @@ -134,6 +141,7 @@ describe( 'applyFormat', () => {
text: 'one two three',
};
const expected = {
activeFormats: [ strong ],
formats: [ , , , [ strong ], [ strong, em ], [ strong, em ], [ em ], , , , , , , ],
text: 'one two three',
};
Expand All @@ -152,6 +160,7 @@ describe( 'applyFormat', () => {
end: 6,
};
const expected = {
activeFormats: [ strong ],
formats: [ , , , [ strong ], [ strong, em ], [ strong, em ], [ em ], , , , , , , ],
text: 'one two three',
start: 3,
Expand Down Expand Up @@ -184,12 +193,14 @@ describe( 'applyFormat', () => {

it( 'should apply format on existing format if selection is collapsed', () => {
const record = {
activeFormats: [ a ],
formats: [ , , , , [ a ], [ a ], [ a ], , , , , , , ],
text: 'one two three',
start: 4,
end: 4,
};
const expected = {
activeFormats: [ a2 ],
formats: [ , , , , [ a2 ], [ a2 ], [ a2 ], , , , , , , ],
text: 'one two three',
start: 4,
Expand Down
2 changes: 2 additions & 0 deletions packages/rich-text/src/test/remove-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe( 'removeFormat', () => {
};
const expected = {
formats: [ , , , , [ em ], [ em ], [ em ], , , , , , , ],
activeFormats: [],
text: 'one two three',
};
const result = removeFormat( deepFreeze( record ), 'strong', 3, 6 );
Expand All @@ -37,6 +38,7 @@ describe( 'removeFormat', () => {
};
const expected = {
formats: [ , , , , [ em ], [ em ], [ em ], , , , , , , ],
activeFormats: [],
text: 'one two three',
};
const result = removeFormat( deepFreeze( record ), 'strong', 4, 4 );
Expand Down
2 changes: 2 additions & 0 deletions packages/rich-text/src/test/toggle-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe( 'toggleFormat', () => {
};
const expected = {
formats: [ , , , , [ em ], [ em ], [ em ], , , , , , , ],
activeFormats: [],
text: 'one two three',
start: 3,
end: 6,
Expand All @@ -43,6 +44,7 @@ describe( 'toggleFormat', () => {
};
const expected = {
formats: [ , , , [ strong ], [ strong, em ], [ strong, em ], [ em ], , , , , , , ],
activeFormats: [ strong ],
text: 'one two three',
start: 3,
end: 6,
Expand Down

0 comments on commit 449c254

Please sign in to comment.