Skip to content

Commit

Permalink
Migrating to CK5: PR2-Initialise CKeditor 5 (oppia#7184)
Browse files Browse the repository at this point in the history
* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Rename the directive to ck-editor-4

* Run CI tests in oppia

* Initialize ck5 with ck4

* Add the flag

* Add code-owners

* Ignore console errors

* Expected by me

* Change username

* Fix merge conflicts

* Resolve comments

* Address comments-2

* Update expected errors in e2e

* Remove ng-touch message from expected

* Add ()

* Address comments

* update language

* Update codeowners and var names

* dev_mode

* Fix-1

* Re-arrange CK

* Update few more directives

* Cross-chck

* Resolve issues

* Address Comments

* Address Comments-2

* Address comments-4

* Remove extra space

* Address comments-1

* Address comments-2

* Address comments and regex

* Remove oppia module export

* Update regex

* Fix lint in assets
  • Loading branch information
Nisheal John authored and seanlip committed Aug 5, 2019
1 parent b33aa9c commit 292391f
Show file tree
Hide file tree
Showing 22 changed files with 6,844 additions and 3,193 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@

# Rich text editor team.
/core/templates/dev/head/components/ck-editor-helpers/ck-editor-4-rte.directive.ts @aks681
/core/templates/dev/head/components/ck-editor-helpers/ck-editor-5-rte.directive.ts @aks681 @NishealJ
/core/templates/dev/head/directives/mathjax-bind.directive.ts @aks681
/core/templates/dev/head/mathjaxConfig.ts @aks681
/core/templates/dev/head/components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts @aks681
Expand Down
4 changes: 4 additions & 0 deletions assets/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ var constants = {
// Whether to allow custom event reporting to Google Analytics.
"CAN_SEND_ANALYTICS_EVENTS": false,

// This specifies the current editor in use and used to switch
// between CK4 & CK5.
"CURRENT_RTE_IS_CKEDITOR_4": true,

"ALL_CATEGORIES": ["Algebra", "Algorithms", "Architecture", "Arithmetic",
"Art", "Astronomy", "Biology", "Business", "Calculus", "Chemistry",
"Combinatorics", "Computing", "Economics", "Education", "Engineering",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright 2018 The Oppia Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS-IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/**
* @fileoverview Directive for CK Editor 5.
*/

require('services/ContextService.ts');
require('services/RteHelperService.ts');

const ClassicEditor = require(
'@ckeditor/ckeditor5-build-classic/build/ckeditor.js');

angular.module('oppia').directive('ckEditor5Rte', [
'ContextService', 'RteHelperService', 'PAGE_CONTEXT',
function(ContextService, RteHelperService, PAGE_CONTEXT) {
return {
restrict: 'E',
scope: {
uiConfig: '&'
},
// This is the temmplate to which the CKE5 should be initalized.
// The first <div> is the container for CKE5, the second <div>
// is for the editor tools bar, all the needed css for toolbar
// should be applied in the second div. The contenteditable <div>
// is the editor text-area.
template: '<div>' +
' <div></div>' +
' <div contenteditable="true" class="oppia-rte"></div>' +
'</div>',
require: '^ngModel',

link: function(scope: ICustomScope, elem, attrs, ngModel) {
var _RICH_TEXT_COMPONENTS = RteHelperService.getRichTextComponents();
var names = [];
var icons = [];
var canUseFs = (
ContextService.getPageContext() === PAGE_CONTEXT.EXPLORATION_EDITOR ||
ContextService.getPageContext() === PAGE_CONTEXT.TOPIC_EDITOR ||
ContextService.getPageContext() === PAGE_CONTEXT.STORY_EDITOR ||
ContextService.getPageContext() === PAGE_CONTEXT.SKILL_EDITOR);
_RICH_TEXT_COMPONENTS.forEach(function(componentDefn) {
var componentRequiresFsButFsCannotBeUsed = (
!canUseFs && componentDefn.requiresFs);
if (!((scope.uiConfig() &&
scope.uiConfig().hide_complex_extensions &&
componentDefn.isComplex) || componentRequiresFsButFsCannotBeUsed)) {
names.push(componentDefn.id);
icons.push(componentDefn.iconDataUrl);
}
});

// Create rules to whitelist all the rich text components and their
// wrappers and overlays. See format of filtering
// rules here: http://docs.ckeditor.com/#!/guide/dev_allowed_content_rules
// Whitelist the component tags with any attributes and classes.
var componentRule = names.map(function(name) {
return 'oppia-noninteractive-' + name;
}).join(' ') + '(*)[*];';
// Whitelist the inline component wrapper, which is a
// span with a "type" attribute.
var inlineWrapperRule = ' span[type];';
// Whitelist the block component wrapper, which is a div
// with a "type" attribute and a CSS class.
var blockWrapperRule = ' div(oppia-rte-component-container)[type];';
// Whitelist the transparent block component overlay, which is
// a div with a CSS class.
var blockOverlayRule = ' div(oppia-rte-component-overlay);';
// Put all the rules together.
var extraAllowedContentRules = (
componentRule +
inlineWrapperRule + blockWrapperRule + blockOverlayRule);

var startupFocusEnabled = true;
if (
scope.uiConfig() &&
scope.uiConfig().startupFocusEnabled !== undefined) {
startupFocusEnabled = scope.uiConfig().startupFocusEnabled;
}
// CkEditor5 is initalized to the editable element which is passed
// through the create api. el[0] is the ck-editor-5-rte and
// el[0].children[0].children[1] is the contenteditable div which
// is defined in the template above.
var ck = ClassicEditor.create(
<HTMLElement>(elem[0].children[0].children[1]));

// A RegExp for matching rich text components.
var componentRegExp = (
/(<(oppia-noninteractive-(.+?))\b[^>]*>)[\s\S]*?<\/\2>/g
);
}
};
}
]);
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<!-- label-for-focus-target feature is removed since CKEditor has a
focusManager to manage the focus activity automatically. -->
<ck-editor-4-rte ng-if="!$ctrl.isDisabled()" ng-model="$ctrl.localValue"
<ck-editor-5-rte ng-if="!$ctrl.isDisabled() && !$ctrl.currentRteIsCKEditor4" ng-model="$ctrl.localValue"
ui-config="$ctrl.uiConfig()">
</ck-editor-5-rte>
<ck-editor-4-rte ng-if="!$ctrl.isDisabled() && $ctrl.currentRteIsCKEditor4" ng-model="$ctrl.localValue"
ui-config="$ctrl.uiConfig()">
</ck-editor-4-rte>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ angular.module('oppia').directive('schemaBasedHtmlEditor', [
'/components/forms/schema-based-editors/' +
'schema-based-html-editor.directive.html'),
controllerAs: '$ctrl',
controller: [function() {}]
controller: ['$scope', 'CURRENT_RTE_IS_CKEDITOR_4',
function($scope, CURRENT_RTE_IS_CKEDITOR_4) {
var ctrl = this;
ctrl.currentRteIsCKEditor4 = CURRENT_RTE_IS_CKEDITOR_4;
}]
};
}]);
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

require('components/ck-editor-helpers/ck-editor-4-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-5-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts');
require('directives/angular-html-bind.directive.ts');
require('directives/mathjax-bind.directive.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ <h3>Review Suggestion</h3>
</angular-html-bind>
</div>
<div ng-show="suggestionEditorIsShown">
<ck-editor-4-rte ng-model="suggestionData.newSuggestionHtml"></ck-editor-4-rte>
<ck-editor-5-rte ng-if="!currentRteIsCKEditor4" ng-model="suggestionData.newSuggestionHtml">
</ck-editor-5-rte>
<ck-editor-4-rte ng-if="currentRteIsCKEditor4" ng-model="suggestionData.newSuggestionHtml">
</ck-editor-4-rte>
<input type="text" class="form-control oppia-review-message" ng-model="summaryMessage" placeholder="Summarize your new changes">
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ angular.module('oppia').factory('SuggestionModalForCreatorDashboardService', [
},
controller: [
'$log', '$scope', '$uibModalInstance', 'SuggestionModalService',
'canReviewActiveThread', 'description', 'newContent',
'oldContent', 'stateName', 'suggestionIsHandled', 'suggestionStatus',
'suggestionType',
'canReviewActiveThread', 'description', 'CURRENT_RTE_IS_CKEDITOR_4',
'newContent', 'oldContent', 'stateName', 'suggestionIsHandled',
'suggestionStatus', 'suggestionType',
function(
$log, $scope, $uibModalInstance, SuggestionModalService,
canReviewActiveThread, description, newContent,
oldContent, stateName, suggestionIsHandled, suggestionStatus,
suggestionType
canReviewActiveThread, description, CURRENT_RTE_IS_CKEDITOR_4,
newContent, oldContent, stateName, suggestionIsHandled,
suggestionStatus, suggestionType
) {
$scope.isNotHandled = !suggestionIsHandled;
$scope.canReject = $scope.isNotHandled;
Expand All @@ -91,6 +91,8 @@ angular.module('oppia').factory('SuggestionModalForCreatorDashboardService', [
$scope.errorMessage = '';
}

$scope.currentRteIsCKEditor4 = CURRENT_RTE_IS_CKEDITOR_4;

$scope.oldContent = oldContent;
$scope.newContent = newContent;
$scope.stateName = stateName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// TODO(vojtechjelinek): this block of requires should be removed after we
// introduce webpack for /extensions
require('components/ck-editor-helpers/ck-editor-4-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-5-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts');
require(
'components/forms/custom-forms-directives/apply-validation.directive.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// TODO(vojtechjelinek): this block of requires should be removed after we
// introduce webpack for /extensions
require('components/ck-editor-helpers/ck-editor-4-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-5-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts');
require('directives/angular-html-bind.directive.ts');
require('directives/mathjax-bind.directive.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ angular.module('oppia').factory('SuggestionModalForExplorationPlayerService', [
resolve: {},
controller: [
'$scope', '$timeout', '$uibModalInstance', 'ExplorationEngineService',
'PlayerPositionService', 'PlayerTranscriptService',
'SuggestionModalService',
'CURRENT_RTE_IS_CKEDITOR_4', 'PlayerPositionService',
'PlayerTranscriptService', 'SuggestionModalService',
function(
$scope, $timeout, $uibModalInstance, ExplorationEngineService,
PlayerPositionService, PlayerTranscriptService,
SuggestionModalService) {
CURRENT_RTE_IS_CKEDITOR_4, PlayerPositionService,
PlayerTranscriptService, SuggestionModalService) {
var stateName = PlayerPositionService.getCurrentStateName();
var displayedCard = PlayerTranscriptService.getCard(
PlayerPositionService.getDisplayedCardIndex());
Expand All @@ -62,6 +62,9 @@ angular.module('oppia').factory('SuggestionModalForExplorationPlayerService', [
$scope.cancelSuggestion = function() {
SuggestionModalService.cancelSuggestion($uibModalInstance);
};

$scope.currentRteIsCKEditor4 = CURRENT_RTE_IS_CKEDITOR_4;

$scope.submitSuggestion = function() {
var data = {
target_id: ExplorationEngineService.getExplorationId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ <h3>Suggest a Change</h3>

<div class="modal-body protractor-test-exploration-suggestion-modal">
<!-- ng-model needs to bind to a property of an object on the scope (the property cannot sit directly on the scope). -->
<ck-editor-4-rte ng-show="showEditor" ng-model="suggestionData.suggestionHtml"></ck-editor-4-rte>
<ck-editor-5-rte ng-if="!currentRteIsCKEditor4" ng-show="showEditor" ng-model="suggestionData.suggestionHtml">
</ck-editor-5-rte>
<ck-editor-4-rte ng-if="currentRteIsCKEditor4" ng-show="showEditor" ng-model="suggestionData.suggestionHtml">
</ck-editor-4-rte>
<br>
Briefly describe your changes (required):
<input type="text" ng-model="description" class="protractor-test-suggestion-description-input" style="width: 100%">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// TODO(vojtechjelinek): this block of requires should be removed after we
// introduce webpack for /extensions
require('components/ck-editor-helpers/ck-editor-4-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-5-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts');
require(
'components/state-directives/answer-group-editor/' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// TODO(vojtechjelinek): this block of requires should be removed after we
// introduce webpack for /extensions
require('components/ck-editor-helpers/ck-editor-4-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-5-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts');
require('filters/convert-unicode-with-params-to-html.filter.ts');
require('filters/convert-html-to-unicode.filter.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// TODO(vojtechjelinek): this block of requires should be removed after we
// introduce webpack for /extensions
require('components/ck-editor-helpers/ck-editor-4-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-5-rte.directive.ts');
require('components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts');
require('filters/convert-unicode-with-params-to-html.filter.ts');
require('filters/convert-html-to-unicode.filter.ts');
Expand Down
1 change: 1 addition & 0 deletions core/tests/protractor/accessibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('Cache Slugs', function() {
it('should check that errors get logged for missing resources', function() {
browser.get('/console_errors');
var expectedErrors = [
'ckeditor-duplicated-modules',
'http://localhost:9001/build/fail/logo/288x128_logo_white.png'
];
general.checkConsoleErrorsExist(expectedErrors);
Expand Down
11 changes: 11 additions & 0 deletions core/tests/protractor_utils/general.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ var checkForConsoleErrors = function(errorsToIgnore) {
return !(browserLog.message.includes(' Slow network is detected.'));
});
}

// Since we are in the process of upgrading CKeditor-4 to Ckeditor-5,
// we are observing consoles errors for duplicated modules
// (for more details,
// please refer to https://github.com/ckeditor/ckeditor5/issues/1821)
// and therefore, we need to filter such logs until we've upgraded to
// CKEditor-5 completely.
browserLogs = browserLogs.filter(function(browserLog) {
return !(browserLog.message.includes('ckeditor-duplicated-modules'));
});

for (var i = 0; i < browserLogs.length; i++) {
if (browserLogs[i].level.value > CONSOLE_LOG_THRESHOLD) {
var errorFatal = true;
Expand Down
Loading

0 comments on commit 292391f

Please sign in to comment.