Skip to content

Commit

Permalink
Merge pull request #8535 from ckeditor/i/8335
Browse files Browse the repository at this point in the history
Fix (html-embed): HTML embed editing UI should not be broken when the editor uses an RTL language. Closes #8335.

Fix (theme-lark): HTML embed editing UI should not be broken when the editor uses an RTL language (see #8335). 

Feature (ui): Added support for a new south-east position of the `TooltipView` (see #8335).

Feature (theme-lark): Added styles for a new south-east position of the `TooltipView` (see #8335).
  • Loading branch information
oleq authored Nov 30, 2020
2 parents b8538de + 7a89ed8 commit 1ac756a
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 23 deletions.
12 changes: 7 additions & 5 deletions packages/ckeditor5-html-embed/src/htmlembedediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ export default class HtmlEmbedEditing extends Plugin {

const viewContainer = writer.createContainerElement( 'div', {
class: 'raw-html-embed',
'data-html-embed-label': t( 'HTML snippet' )
'data-html-embed-label': t( 'HTML snippet' ),
dir: editor.locale.uiLanguageDirection
} );
// Widget cannot be a raw element because the widget system would not be able
// to add its UI to it. Thus, we need this wrapper.
Expand Down Expand Up @@ -252,7 +253,7 @@ export default class HtmlEmbedEditing extends Plugin {
sanitizeHtml: props.sanitizeHtml
};

domElement.append( createPreviewContainer( { domDocument, state, props: previewContainerProps } ) );
domElement.append( createPreviewContainer( { domDocument, state, props: previewContainerProps, editor } ) );
} else {
const textareaProps = {
isDisabled: true,
Expand Down Expand Up @@ -323,9 +324,10 @@ export default class HtmlEmbedEditing extends Plugin {
return domTextarea;
}

function createPreviewContainer( { domDocument, state, props } ) {
function createPreviewContainer( { domDocument, state, props, editor } ) {
const domPreviewContainer = createElement( domDocument, 'div', {
class: 'raw-html-embed__preview'
class: 'raw-html-embed__preview',
dir: editor.locale.contentLanguageDirection
} );

const sanitizeOutput = props.sanitizeHtml( state.getRawHtmlValue() );
Expand All @@ -347,7 +349,7 @@ function createDomButton( editor, type ) {
const command = editor.commands.get( 'updateHtmlEmbed' );

buttonView.set( {
tooltipPosition: 'sw',
tooltipPosition: editor.locale.uiLanguageDirection === 'rtl' ? 'se' : 'sw',
icon: pencilIcon,
tooltip: true
} );
Expand Down
77 changes: 77 additions & 0 deletions packages/ckeditor5-html-embed/tests/htmlembedediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,26 @@ describe( 'HtmlEmbedEditing', () => {
expect( getModelData( model ) ).to.equal( '<rawHtml value="foo"></rawHtml>[<rawHtml value="bar"></rawHtml>]' );
} );

describe( 'different setting of ui language', () => {
it( 'the widget should have the dir attribute for LTR language', () => {
sinon.stub( editor.locale, 'uiLanguageDirection' ).value( 'ltr' );

setModelData( model, '<rawHtml></rawHtml>' );
const widget = viewDocument.getRoot().getChild( 0 );

expect( widget.getAttribute( 'dir' ) ).to.equal( 'ltr' );
} );

it( 'the widget should have the dir attribute for RTL language', () => {
sinon.stub( editor.locale, 'uiLanguageDirection' ).value( 'rtl' );

setModelData( model, '<rawHtml></rawHtml>' );
const widget = viewDocument.getRoot().getChild( 0 );

expect( widget.getAttribute( 'dir' ) ).to.equal( 'rtl' );
} );
} );

describe( 'rawHtmlApi.makeEditable()', () => {
it( 'makes the textarea editable', () => {
setModelData( model, '<rawHtml value="foo"></rawHtml>' );
Expand Down Expand Up @@ -646,6 +666,63 @@ describe( 'HtmlEmbedEditing', () => {

expect( domContentWrapper.querySelector( 'div.raw-html-embed__preview' ).innerHTML ).to.equal( 'bar' );
} );

describe( 'different setting of ui and content language', () => {
it( 'the widget and preview should have the dir attribute for LTR language', () => {
sinon.stub( editor.locale, 'uiLanguageDirection' ).value( 'ltr' );
sinon.stub( editor.locale, 'contentLanguageDirection' ).value( 'ltr' );

setModelData( model, '<rawHtml></rawHtml>' );
const widget = viewDocument.getRoot().getChild( 0 );
const domPreview = getDomPreview( widget );

expect( widget.getAttribute( 'dir' ) ).to.equal( 'ltr' );
expect( domPreview.getAttribute( 'dir' ) ).to.equal( 'ltr' );
} );

it( 'the widget and preview should have the dir attribute for RTL language', () => {
sinon.stub( editor.locale, 'uiLanguageDirection' ).value( 'rtl' );
sinon.stub( editor.locale, 'contentLanguageDirection' ).value( 'rtl' );

setModelData( model, '<rawHtml></rawHtml>' );
const widget = viewDocument.getRoot().getChild( 0 );
const domPreview = getDomPreview( widget );

expect( widget.getAttribute( 'dir' ) ).to.equal( 'rtl' );
expect( domPreview.getAttribute( 'dir' ) ).to.equal( 'rtl' );
} );

it( 'the widget should have the dir attribute for LTR language, but preview for RTL', () => {
sinon.stub( editor.locale, 'uiLanguageDirection' ).value( 'ltr' );
sinon.stub( editor.locale, 'contentLanguageDirection' ).value( 'rtl' );

setModelData( model, '<rawHtml></rawHtml>' );
const widget = viewDocument.getRoot().getChild( 0 );
const domPreview = getDomPreview( widget );

expect( widget.getAttribute( 'dir' ) ).to.equal( 'ltr' );
expect( domPreview.getAttribute( 'dir' ) ).to.equal( 'rtl' );
} );

it( 'the widget should have the dir attribute for RTL language, butPreview for LTR', () => {
sinon.stub( editor.locale, 'uiLanguageDirection' ).value( 'rtl' );
sinon.stub( editor.locale, 'contentLanguageDirection' ).value( 'ltr' );

setModelData( model, '<rawHtml></rawHtml>' );
const widget = viewDocument.getRoot().getChild( 0 );
const domPreview = getDomPreview( widget );

expect( widget.getAttribute( 'dir' ) ).to.equal( 'rtl' );
expect( domPreview.getAttribute( 'dir' ) ).to.equal( 'ltr' );
} );

function getDomPreview( widget ) {
const contentWrapper = widget.getChild( 1 );
const domContentWrapper = editor.editing.view.domConverter.mapViewToDom( contentWrapper );

return domContentWrapper.querySelector( 'div.raw-html-embed__preview' );
}
} );
} );
} );
} );
Expand Down
8 changes: 8 additions & 0 deletions packages/ckeditor5-html-embed/tests/manual/htmlembed.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
<input type="radio" id="raw-html-previews-disabled" name="mode" value="disabled"><label for="raw-html-previews-disabled"><code>false</code></label>
</p>

<p>
<b>Language direction</b>:
<input type="radio" id="language-en" name="language" value="en" checked><label for="language-en"></label>
<input type="radio" id="language-ar" name="language" value="ar"><label for="language-ar"></label>
<input type="radio" id="language-en-ar" name="language" value="en-ar"><label for="language-en-ar">UI: ↦, Content: ↤</label>
<input type="radio" id="language-ar-en" name="language" value="ar-en"><label for="language-ar-en">UI: ↤, Content: ↦</label>
</p>

<div id="editor">
<h3><code>&lt;video&gt;</code> tag as HTML snippet</h3>
<div class="raw-html-embed">
Expand Down
36 changes: 25 additions & 11 deletions packages/ckeditor5-html-embed/tests/manual/htmlembed.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,35 @@ const noPreviewsModeButton = document.getElementById( 'raw-html-previews-disable
previewsModeButton.addEventListener( 'change', handleModeChange );
noPreviewsModeButton.addEventListener( 'change', handleModeChange );

startMode( document.querySelector( 'input[name="mode"]:checked' ).value );
for ( const input of document.querySelectorAll( 'input[name="language"]' ) ) {
input.addEventListener( 'change', handleModeChange );
}

startMode();

async function handleModeChange( evt ) {
await startMode( evt.target.value );
async function handleModeChange() {
await startMode();
}

async function startMode( selectedMode ) {
async function startMode() {
const selectedMode = document.querySelector( 'input[name="mode"]:checked' ).value;
const [ uiLanguage, contentLanguage ] = document.querySelector( 'input[name="language"]:checked' ).value.split( '-' );

const language = {
ui: uiLanguage,
content: contentLanguage || uiLanguage
};

if ( selectedMode === 'enabled' ) {
await startEnabledPreviewsMode();
await startEnabledPreviewsMode( { language } );
} else {
await startDisabledPreviewsMode();
await startDisabledPreviewsMode( { language } );
}
}

async function startEnabledPreviewsMode() {
async function startEnabledPreviewsMode( config ) {
await reloadEditor( {
...config,
htmlEmbed: {
showPreviews: true,
sanitizeHtml( rawHtml ) {
Expand All @@ -49,16 +62,17 @@ async function startEnabledPreviewsMode() {
} );
}

async function startDisabledPreviewsMode() {
await reloadEditor();
async function startDisabledPreviewsMode( config ) {
await reloadEditor( config );
}

async function reloadEditor( config = {} ) {
if ( window.editor ) {
await window.editor.destroy();
}

config = Object.assign( config, {
config = {
...config,
plugins: [ ArticlePluginSet, HtmlEmbed, Code ],
toolbar: [
'heading', '|', 'bold', 'italic', 'link', '|',
Expand All @@ -68,7 +82,7 @@ async function reloadEditor( config = {} ) {
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]
}
} );
};

window.editor = await ClassicEditor.create( document.querySelector( '#editor' ), config );
}
Expand Down
3 changes: 3 additions & 0 deletions packages/ckeditor5-html-embed/theme/htmlembed.css
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,7 @@
/* Give the html embed some minimal width in the content to prevent them
from being "squashed" in tight spaces, e.g. in table cells (https://github.com/ckeditor/ckeditor5/issues/8331) */
min-width: 15em;

/* Don't inherit the style, e.g. when in a block quote. */
font-style: normal;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@
outline: var(--ck-html-embed-unfocused-outline-width) dashed var(--ck-color-widget-blurred-border);
}

& .ck-widget__type-around .ck-widget__type-around__button.ck-widget__type-around__button_before {
margin-left: 50px;
/* HTML embed widget itself should respect UI language direction */
&[dir="ltr"] {
text-align: left;
}

&[dir="rtl"] {
text-align: right;
}

/* ----- Emebed label in the upper left corner ----------------------------------------------- */
/* ----- Embed label in the upper left corner ----------------------------------------------- */

&::before {
content: attr(data-html-embed-label);
Expand All @@ -38,6 +43,16 @@
font-family: var(--ck-font-face);
}

&[dir="rtl"]::before {
left: auto;
right: var(--ck-spacing-standard);
}

/* Make space for label but it only collides in LTR languages */
&[dir="ltr"] .ck-widget__type-around .ck-widget__type-around__button.ck-widget__type-around__button_before {
margin-left: 50px;
}

@nest .ck.ck-editor__editable.ck-blurred &.ck-widget_selected::before {
top: 0px;
padding: var(--ck-spacing-tiny) var(--ck-spacing-small);
Expand Down Expand Up @@ -78,6 +93,11 @@
}
}

&[dir="rtl"] .raw-html-embed__buttons-wrapper {
left: var(--ck-spacing-standard);
right: auto;
}

/* The edit source element. */
& .raw-html-embed__source {
box-sizing: border-box;
Expand All @@ -92,6 +112,10 @@
white-space: pre-wrap;
font-size: var(--ck-font-size-base); /* Safari needs this. */

/* HTML code is direction–agnostic. */
text-align: left;
direction: ltr;

&[disabled] {
background: hsl(0deg 0% 97%);
color: hsl(0deg 0% 56%);
Expand All @@ -117,4 +141,3 @@
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@
* +-----------+
*/
&.ck-tooltip_s,
&.ck-tooltip_sw {
&.ck-tooltip_sw,
&.ck-tooltip_se {
bottom: calc(-1 * var(--ck-tooltip-arrow-size));
transform: translateY( 100% );

Expand Down Expand Up @@ -96,6 +97,31 @@
}
}

/**
* A class that displays the tooltip south-east of the element.
*
* [element]
* ^
* +-----------+
* | Tooltip |
* +-----------+
*/
&.ck-tooltip_se {
left: 50%;
right: auto;

& .ck-tooltip__text {
right: auto;
left: calc( -2 * var(--ck-tooltip-arrow-size));
}

& .ck-tooltip__text::after {
right: auto;
left: 0;
transform: translateX( 50% );
}
}

/**
* A class that displays the tooltip north of the element.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,16 @@
}
}

/*
* Styles for the "before" button when the widget has a selection handle in an RTL environment.
* The selection handler is aligned to the right side of the widget so there is no need to create
* additional space for it next to the "before" button.
*/
.ck[dir="rtl"] .ck-widget.ck-widget_with-selection-handle .ck-widget__type-around > .ck-widget__type-around__button_before {
margin-left: 0;
margin-right: 20px;
}

/*
* Hide type around buttons when the widget is selected as a child of a selected
* nested editable (e.g. mulit-cell table selection).
Expand Down
10 changes: 8 additions & 2 deletions packages/ckeditor5-ui/src/tooltip/tooltipview.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default class TooltipView extends View {
this.set( 'text', '' );

/**
* The position of the tooltip (south, south-west, or north).
* The position of the tooltip (south, south-west, south-east, or north).
*
* +-----------+
* | north |
Expand All @@ -52,9 +52,15 @@ export default class TooltipView extends View {
* | south west |
* +--------------+
*
* [element]
* ^
* +--------------+
* | south east |
* +--------------+
*
* @observable
* @default 's'
* @member {'s'|'sw'|'n'} #position
* @member {'s'|'sw'|'se'|'n'} #position
*/
this.set( 'position', 's' );

Expand Down

0 comments on commit 1ac756a

Please sign in to comment.