Feature/rich text editor #312

Mboulianne · 2018-06-29T15:42:12Z

`@ulaval/modul-components`

PR Checklist

Provide a small description of the changes introduced by this PR

Include links to issues
https://jira.dti.ulaval.ca/browse/MODUL-152
Openshift deployment requested

Include this section in the release notes

Other info

…ove them

…c loading later)

…sion, in might change in the future) review

gabra20 · 2018-07-16T13:27:21Z

src/components/rich-text-edit/adapter/vue-froala.ts

+    }
+
+    private updateValue(): void {
+        if (JSON.stringify(this.oldModel) === JSON.stringify(this.model)) {


personnellement, j'inverserais la condition du if et je mettrais le setContent a l’intérieur. Ça éviterait un return "vide".

gabra20 · 2018-07-16T14:01:14Z

src/components/rich-text-edit/rich-text-edit.ts

+    @Prop()
+    public scrollableContainer: string;
+
+    protected id: string = `mTextarea-${uuid.generate()}`;


modifier l'id pour 'mRichTextEditor-${uuid.generate()}'

gabra20 · 2018-07-16T14:19:40Z

src/mixins/input-state/input-state.ts

+    public getInput(): HTMLElement | null {
+        const selector: string = this.as<InputStateInputSelector>()!.selector || 'input, textarea, [contenteditable=true]';
+        const elements: NodeListOf<Element> = this.$el.querySelectorAll(selector);
+        if (elements.length > 1 || elements.length === 0) {


if(elements.length != 1)

gabra20 · 2018-07-27T13:08:15Z

src/utils/filter/htmlFilter.ts

+}
+
+export function replaceTags(tags: string[], replace: string, html: string): string {
+    tags.forEach((tag) => { html = replaceTag(tag, replace, html); });


déjà fait mais pas encore pushé

gabra20 · 2018-07-27T13:09:22Z

src/utils/filter/htmlFilter.ts

@@ -0,0 +1,44 @@
+/* tslint:disable:no-console */
+
+export function filterNewLines(html: string): string {


pas bête! je vais tester ça!

gabra20 · 2018-07-27T13:13:51Z

src/components/validation-message/validation-message.html

                </m-icon><span class="m-validation-message__text">{{ errorMessage }}</span>
            </p>
            <p class="m-validation-message__valid" v-if="hasValidMessage && isValid">
-                <m-icon name="check" :svg-title="titleValidIcon" class="m-validation-message__valid__icon"></m-icon>
+                <m-icon name="m-svg__confirmation" :svg-title="titleValidIcon" class="m-validation-message__valid__icon"></m-icon>


D’après le git blame, ça fait parti de l’intégration

gabra20 · 2018-07-27T13:23:49Z

src/components/rich-text/rich-text.ts

+import { RICH_TEXT } from '../component-names';
+import WithRender from './rich-text.html?style=./rich-text.scss';
+
+require('froala-editor/css/froala_editor.pkgd.min.css');


Si on le mets pas l'affichage "riche" se fait pas.

gabra20 · 2018-07-27T13:29:32Z

src/components/rich-text-edit/adapter/vue-froala.ts

+
+function cleanHtml(html: string): string {
+    // delete new lines so the regexps will work
+    html = filterNewLines(html);


je vais regarder ca. Je pense que ca marchera pour les filters mais pas les replace et erase. Je pense que le comportement de la methode est le meme que l'option pasteDeniedTags : "Tags that are removed together with their content". Et on veut pas toujours effacer le contenu.

gabra20 · 2018-07-27T13:30:16Z

src/components/rich-text-edit/adapter/vue-froala.ts

+
+function cleanHtml(html: string): string {
+    // delete new lines so the regexps will work
+    html = filterNewLines(html);


et j'utiliserai l'option si le comportement est le même

gabra20 · 2018-07-27T13:31:44Z

src/components/rich-text-edit/adapter/vue-froala.ts

+    return html;
+}
+
+class PopupPlugin {


je vais le faire!

gabra20 · 2018-07-30T19:34:03Z

src/components/index.ts

@@ -116,6 +121,8 @@ const ComponentsPlugin: PluginObject<any> = {
        Vue.use(RadioPlugin);
        Vue.use(RadioGroupPlugin);
        Vue.use(RadioStylePlugin);
+        Vue.use(RichTextLicensePlugin, { key: options.richTextOptions ? options.richTextOptions.key : undefined });
+        Vue.use(RichTextPlugin); // TODO : rich-text read-only mode will always be included.  Are we all ok with that?


c’était bon pour Vincent, le lecteur est léger.

This reverts commit 4713075.

simardo · 2018-07-27T13:28:03Z

src/components/component-names.ts

@@ -43,6 +43,8 @@ export const PROGRESS_NAME: string = 'm-progress';
 export const RADIO_NAME: string = 'm-radio';
 export const RADIO_GROUP_NAME: string = 'm-radio-group';
 export const RADIO_STYLE_NAME: string = 'm-radio-style';
+export const RICH_TEXT: string = 'm-rich-text';


Ajouter le suffixe _NAME.

simardo · 2018-07-27T13:31:16Z

src/components/index.ts

@@ -116,6 +121,8 @@ const ComponentsPlugin: PluginObject<any> = {
        Vue.use(RadioPlugin);
        Vue.use(RadioGroupPlugin);
        Vue.use(RadioStylePlugin);
+        Vue.use(RichTextLicensePlugin, { key: options.richTextOptions ? options.richTextOptions.key : undefined });
+        Vue.use(RichTextPlugin); // TODO : rich-text read-only mode will always be included.  Are we all ok with that?


Ce fichier est destiné à mourir et le bundling des composantes sera à revoir. Donc je ne me casserais pas trop le becyk avec ça pour le moment.

simardo · 2018-07-31T13:43:07Z

src/components/input-style/input-style.scss

            min-height: 24px;
            flex: 1 1 auto;
+            // overflow: hidden;


enlever les commentaires s'ils ne sont pas nécessaires. Sinon mettre un commentaire sur le pourquoi on veut le garder (ex: à tester dans telle cas particulier)

simardo · 2018-07-31T13:45:37Z

src/components/input-style/input-style.ts

@@ -2,7 +2,7 @@ import { PluginObject } from 'vue';
 import Component from 'vue-class-component';
 import { Prop } from 'vue-property-decorator';

-import { InputState } from '../../mixins/input-state/input-state';
+import { InputState, InputStateMixin } from '../../mixins/input-state/input-state';


J'ai mal à la tête avec tous ces input-style-state-management-mixin qui s'entremêlent!!!

je remets InputState partout, je demanderai a @Mboulianne s'il y avait une raison particulière à utiliser InputStateMixin quand il rentrera de vacances.

C'était un commentaire gratuit de ma part :) C'est juste qu'avec le temps, j'ai complètement perdu le contrôle de toutes ces input-* mixins/state/etc. Ça se peut que ça soit bien correct de même.

D'apres mes tests, ca marche que l'on utilise InputState ou InputStateMixin dans les as, puisque InputState implemente InputStateMixin. Donc, autant utiliser InputState partout je pense. En tout cas c'est fait ;-)

simardo · 2018-07-31T13:50:33Z

src/components/rich-text-edit/adapter/popup-plugin.ts

@@ -0,0 +1,51 @@
+export class PopupPlugin {
+    private editor: any;


La déclaration des attributs ici n'est pas nécessaire si on utilise le constructeur comme ceci:

constructor(private editor, private name: string, private pluginName: string, private buttonList: string[]) { // this.editor = editor; plus nécessaire this.buttonName = `${name}Popup`; this.pluginName = `${name}Plugin`; // this.buttonList = buttonList; plus nécessaire }

Si on ne veut pas passer le pluginName en paramètre, on peut le premettre comme attribut. Au choix. Ça peut être laissé tel quel aussi, j'ai pas de préférences.

simardo · 2018-07-31T13:51:15Z

src/components/rich-text-edit/adapter/popup-plugin.ts

+        this.editor.popups.show(`${this.pluginName}.popup`, left, top, btn.outerHeight());
+    }
+
+    hidePopup(): void {


Mettre public

simardo · 2018-07-31T13:58:43Z

src/components/rich-text-edit/adapter/popup-plugin.ts

+    private pluginName: string;
+    private buttonList: string[];
+
+    constructor(editor, name: string, buttonList: string[]) {


editor: any
Est-ce qu'on a envisagé faire un d.ts de l'éditeur? du moins pour les propriétés et méthodes utilisées? Ça devient beaucoup plus simple pour la personne qui a à maintenir le code, surtout si elle n'a pas vraiment contribué à la composante.

simardo · 2018-07-31T14:02:13Z

src/components/rich-text-edit/adapter/vue-froala.ts

+
+    protected currentTag: string = 'div';
+    protected listeningEvents: any[] = [];
+    protected froalaEditor: any = undefined;


Beaucoup de any. Est-ce que ça aurait été relativement simple de les typer selon la documentation qu'on a et l'utilisation qu'on en fait?

Ce code a été en grande partie de https://github.com/froala/vue-froala-wysiwyg. On l'a pas encore trop optimisé. C'est un bon point, on créera tous les types que l'on peut créer, mais je pense qu'il restera quand même quelques any (froala est codé en javascript + jQuery).

simardo · 2018-07-31T14:08:29Z

src/components/rich-text-edit/rich-text-edit.ts

+
+}
+
+export class RichTextLicensePlugin implements PluginObject<RichTextLicensePluginOptions | undefined> {


Ce code se retrouve à deux places. Ici et dans rich-text-license-plugin

simardo · 2018-07-31T14:20:25Z

package.json

@@ -82,7 +87,7 @@
        "vue-class-component": "^6.1.1",
        "vue-property-decorator": "^6.0.0",
        "vue-template-compiler": "^2.5.9",
-        "vue-template-loader": "^0.3.1"
+        "vue-template-loader": "^1.0.0"


Bien que ce soit une dev-dependency, je ne suis pas certain que c'est cette dépendance qui sera prise en compte dans Prisme si elle est redéfinie dans le package.json. C'était peut-être un bon cas de peer-dependency finalement!

simardo · 2018-07-31T14:25:40Z

package-lock.json

            }
        },
        "@mrmlnc/readdir-enhanced": {
            "version": "2.2.1",
-            "resolved": "https://registry.npmjs.org/@mrmlnc/readdir-enhanced/-/readdir-enhanced-2.2.1.tgz",
-            "integrity": "sha512-bPHp6Ji8b41szTOcaP63VlnbbO5Ny6dwAATtY6JTjh5N2OLrb5Qk/Th5cRkRQhkWCt+EJsYrNB0MiL+Gpn6e3g==",
+            "resolved": "https://maven.ulaval.ca/api/npm/npm/@mrmlnc/readdir-enhanced/-/readdir-enhanced-2.2.1.tgz",


Ne doit pas avoir de références Maven sur github. Si ça ne fonctionne pas de votre bord, je peux le regénérer avant de merger.

simardo · 2018-08-01T13:35:19Z

src/components/rich-text-edit/adapter/vue-froala.ts

@@ -93,7 +93,7 @@ export class VueFroala extends Vue {
        $.FroalaEditor.POPUP_TEMPLATES[`${pluginName}.popup`] = '[_BUTTONS_]';

        // The custom popup is defined inside a plugin (new or existing).
-        $.FroalaEditor.PLUGINS[pluginName] = (editor) => { return new PopupPlugin(editor, name, buttonList); };
+        $.FroalaEditor.PLUGINS[pluginName] = (editor) => { return new PopupPlugin(name, editor, buttonList); };


Juste pour être certain... il y a eu ici une inversion des champs nom et editor dans le dernier commit. C'est voulu?

Oublie ça... l'ordre a changé dans le constructeur!

Mboulianne and others added 24 commits June 13, 2018 14:12

RTE poc #2

4c59d2d

faire fonctionner fontawesome

1a6a601

enable to provide rich-text license through plugin options

6f67dbe

review

b0eb7b5

Code review

4fb60bc

Do some cleanup

d1c69f0

add a readonly switch to rich text editor

792db13

Remove global styles for LI... still need to make sure it's ok to rem…

33ddfee

…ove them

Implement rich-text-editor read component

ac9a0f0

rich-text-editor extract a read and a write component (to enable asyn…

eca65e7

…c loading later)

put rich-text-editor edit mode in a separate webpack chunk

259b81b

Make a new vue-froala component

953ac1a

enable webpack BundleAnalyzerPlugin te be able to visualize modUL chunks

16921ef

modul library will always output 1 webpack chunk max (to avoid regres…

c6d2a85

…sion, in might change in the future) review

Code review

e9ea28a

begin refactor

0184536

Remove superfluous changes

29dd19e

Comment vue-froala

d19aeed

Rename, review, etc

600ba91

load only icons that we need for the editor

426af86

fix richtext plugin options

43bfca2

add undefined to richtextplugin options

dc40260

setup rich-text options

e944fbc

refactor

cd099a0

Mboulianne added help wanted new feature New component, directive or filter labels Jun 29, 2018

Mboulianne requested review from jfnadeau, simardo and vidal7 as code owners June 29, 2018 15:42

setur52 approved these changes Jul 30, 2018

View reviewed changes

gabra20 added 5 commits July 30, 2018 10:46

corrected comment

4678b24

create unitary tests

5d66a56

set silent back on unitwatch

20bbf1d

clean sandbox

2922c4f

set standard mode in standard mode test

34f48f3

gabra20 approved these changes Jul 30, 2018

View reviewed changes

gabra20 added 3 commits July 30, 2018 15:16

merge develop

719c2b2

corrected package-lock

b094ced

updated package-lock

4713075

gabra20 approved these changes Jul 30, 2018

View reviewed changes

gabra20 added 3 commits July 30, 2018 15:59

Revert "updated package-lock"

14248db

This reverts commit 4713075.

corrected package-lock

f61677e

set fullscreen on initialisation for mobiles

ddfa1fb

simardo suggested changes Jul 31, 2018

View reviewed changes

simardo reviewed Jul 31, 2018

View reviewed changes

gabra20 added 8 commits July 31, 2018 10:35

add NAME to component-names constant

56ed58e

optimised popup-plugin constructor

e6f10ba

delete todo comment

563984b

add public to popup-plugin methods

7b318ac

delete richTextLicencePlugin doublon

269876f

corrected package-lock

2e396a1

deleted comment

b8d7bf2

use only InputState in input-style

e8f3885

simardo reviewed Aug 1, 2018

View reviewed changes

simardo approved these changes Aug 1, 2018

View reviewed changes

deleted background color from input-style

ef33481

simardo merged commit 8ab122f into develop Aug 1, 2018

simardo deleted the feature/rich-text-editor branch August 1, 2018 14:30

simardo added this to the 1.0.0-beta.44 milestone Aug 1, 2018

		@@ -0,0 +1,44 @@
		/* tslint:disable:no-console */

		export function filterNewLines(html: string): string {

		@@ -0,0 +1,51 @@
		export class PopupPlugin {
		private editor: any;


		}

		export class RichTextLicensePlugin implements PluginObject<RichTextLicensePluginOptions \| undefined> {

Feature/rich text editor #312

Feature/rich text editor #312

Uh oh!

Conversation

Mboulianne commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

@ulaval/modul-components

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabra20 Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mboulianne commented Jun 29, 2018 •

edited

Loading

`@ulaval/modul-components`

gabra20 Jul 31, 2018 •

edited

Loading