Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/51: Underline #54

Merged
merged 3 commits into from
Aug 28, 2017
Merged

T/51: Underline #54

merged 3 commits into from
Aug 28, 2017

Conversation

pomek
Copy link
Member

@pomek pomek commented Aug 22, 2017

Suggested merge commit message (convention)

Feature: Introduced the Underline plugin. Closes ckeditor/ckeditor5#5545.

@pomek pomek requested a review from Reinmar August 22, 2017 11:21
@Reinmar
Copy link
Member

Reinmar commented Aug 22, 2017

You need to update contexts.json.

const editor = this.editor;
const t = editor.t;
const command = editor.commands.get( 'underline' );
const keystroke = 'CTRL+U';
Copy link
Member

@Reinmar Reinmar Aug 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this. Does CKE4 have some shortcut? Is it common for Ctrl+U to make underline? cc @oleq @Comandeer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there's shortcut for it in CKE4 – Ctrl+U. The same shortcut is also available in Word, so it's probably safe to suppose that it's a de facto standard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Pages CMD+U also works as I expected (makes underline).

@pomek
Copy link
Member Author

pomek commented Aug 22, 2017

You need to update contexts.json.

Done in e14259d.

Another question related to translation – should I do anything with translations files?

@Reinmar Reinmar requested a review from szymonkups August 22, 2017 14:02
@Reinmar
Copy link
Member

Reinmar commented Aug 22, 2017

Another question related to translation – should I do anything with translations files?

Nope. Uploading and downloading data to/from Transifex is done by me.

*/

/**
* @module basic-styles/underline
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@module basic-styles/underlineengine

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2a1e748.

@szymonkups szymonkups merged commit f724ae0 into master Aug 28, 2017
@szymonkups szymonkups deleted the t/51 branch August 28, 2017 09:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Underline
4 participants