Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Froala 3 fixes #59

Closed
wants to merge 17 commits into from
Closed

Froala 3 fixes #59

wants to merge 17 commits into from

Conversation

alexisszabo
Copy link

@alexisszabo alexisszabo commented Jul 10, 2019

Thanks for putting together this component. Made it nice and easy to integrate the froala-editor in v2.9.5. After upgrading to v3, I noticed that a number of things were no longer working. Since I needed this urgently for a project I am working on, I made some updates to aurelia-froala-editor to work nicely with v3. If you approve, can this be merged into the mainstream? Please let me know if any questions or edits needed.

This pull request:

  • Fixes event handlers
  • Fixes binding of value
  • Adds 'editor' as a bindable value in order to set settings in the parent component
  • Setup editor on correct component (either div or textarea depending on iframe setting)
  • Refactors for additional clarity

Please note, I did not test with the iframe setting set to true.

Alexis Szabo added 13 commits July 4, 2019 12:08
Since component binding happend before attaching, we should not start listening
for changes in value until the component has been attached.
Comments were using spaces, code was using tabs. Standardized on tabs
This is useful if we need access the instance after initialization
'data-froala.editor' no longer seems to be used in Froala 3
@alexisszabo
Copy link
Author

I updated the pull request to match the changes in 3.0.3-2

@kapil2704
Copy link
Contributor

kapil2704 commented Nov 1, 2019

Hi @alexisszabo ,

Could you please Update your PR for this to be merged soon ? also please write purposes in more detail.

Thanks,
Kapil Garg
Froala Architect

@@ -36,7 +36,7 @@
"babel-preset-stage-1": "^6.22.0",
"conventional-changelog": "^2.0.3",
"del": "^3.0.0",
"gulp": "^4.0.0",
"gulp": "^3.9.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

but we can run those tasks with version 4. isn't it?

Copy link
Author

@alexisszabo alexisszabo Nov 19, 2019

Choose a reason for hiding this comment

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

Hi @kapil2704 , the gulp tasks were written with the 3.x syntax which fails with gulp 4. Since I was trying to minimize the changes, I rolled back the gulp version to 3.x instead of rewriting tasks in the 4.x syntax. Please see a related issue here: #60 (comment)

@@ -101,7 +101,7 @@ define(['exports', 'aurelia-framework', 'aurelia-binding', './froala-editor-conf
}
})];

this.editor = new _froala_editorPkgdMin2.default('#' + this.element.id + ' ' + editorSelector, Object.assign({}, this.config), function () {
this.editor = new _froala_editorPkgdMin2.default(this.element.querySelector(editorSelector), Object.assign({}, this.config), function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

it will select first element of that type. not that particular element?

Copy link
Author

@alexisszabo alexisszabo Nov 19, 2019

Choose a reason for hiding this comment

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

That's correct. However, for example, since we are searching only within the element itself, and there is only one 'textarea', we will get that.

This is similar to the old code:
if (this.config.iframe) {
this.instance = this.element.getElementsByTagName('textarea')[0];
}
else {
this.instance = this.element.getElementsByTagName('div')[0];
}

@alexisszabo
Copy link
Author

alexisszabo commented Nov 19, 2019

Hi @alexisszabo ,

Could you please Update your PR for this to be merged soon ? also please write purposes in more detail.

Thanks,
Kapil Garg
Froala Architect

@kapil2704 I updated the PR to use version 3.0.6 of the froala-editor. Also resolved merge conflicts. Is more detail needed than what is in #59 (comment) ?

@alexisszabo
Copy link
Author

HI @kapil2704 , Can we get an update the on the status of v3 support?

Thanks,
Alexis

@alexisszabo
Copy link
Author

Closed PR. Old and better to fix the latest official version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants