An alternate addon, combine forces

[Panman82]

I just wanted to make you aware of an alternate Froala Editor addon that I've created. It started out as a way to propose some changes here but I ended up running with it. https://github.com/Panman8201/ember-froala-editor

TLDR; Let's figure out a way to combine this stuff into one addon

Here is a list of the main differences;

• content over value property name, keeps it in-line with what the Froala Editor calls it (with the contentChanged event name anyway)
• Support binding the content property to a Froala Editor event, contentChanged by default
• options over params property name, made more sense to call it "options" and keeps it in-line with what Froala Editor actually calls them
• Support for defaultOptions (although I have plans to consolidate it into just the options property, just waiting on a Ember.js bugfix to be released)
• Better support for handling event actions
  • Prefixing on-* eliminates the possibility of attaching a "non-event" listener (ex: {{froala-editor class="foobar"}})
  • Appending *-getHtml plays nicely with the ember (mut) helper
  • Support for string based action names, although not the recommendation now days (ex: {{froala-editor on-blur="someAction"}})
  • Support for canceling events by passing false from the action (when supported by the Froala Editor that is...)
  • Properly removes event listeners (if not done, known to cause memory leaks)
  • Sends a component instance to the action, in place of the editor instance
• Moved custom buttons, plugins, etc. to an ember initializer instead of the component instance, will be properly created once that way...
• Relies on the ember-cli-font-awesome addon (and will add a project devDependancy for it) instead of re-inventing the wheel
• The addon index.js importing options are very flexible
  • Full word (instead of abbreviation) for better clarity
  • Every option can import multiple files (including themes and languages)
  • Addon doesn't need to maintain a list of plugins (uses the filesystem to detect what's available)
• {{froala-editor}} is very aware of its current state, being careful not to call handlers and such at the wrong time

And beyond the differences, here are some of the added features;

• Ability to define defaultContent, which itself defaults to <p><br></p> (which in relation is the actual Froala Editor's default)
• Ember SafeString support; SafeString in, SafeString out
• (untested) Ember Fastboot support (by using the didRender() hook over didInsertElement() hook)
• Support for changing the content and options properties (will properly update the editors HTML for content changes and reinitialize for option changes)
• Exposed Froala Editor methods in ember-land via component action and method
• {{froala-content}} component to wrap generated HTML in the .fr-view class (as suggested inthe Froala Editor docs)
• (froala-method) helper to wire an editor event and method together
• (merged-hash) helper which can be very handy with the options
• Docs Website that matches the Froala Website (responsive and print-friendly)
• And probably more I'm just not thinking of...

So, where am I going with this? I'd like join forces and merge this stuff together into one addon. My main concern for not providing PR's are the breaking changes (property name differences, event handling altogether, index.js option names, etc.). I'm more than willing to add all the existing contributors and transfer ownership of this repo to the froala org. Or do you think there is a better way forward? Or perhaps our opinions are too different to join together.?. Let's discuss.

P.S. I'm honestly not trying to step on anyone's toes here, but seen some things with the existing addon that concerned me, which lead me down this road...

[stefanneculai]

@Panman8201 👍

@artsmc @maulsea @jigneshchudasama05 would love to hear your thoughts about this!

[artsmc]

Yeah sounds great to me I'm actually reading it over now.
@Panman8201 👍

[artsmc]

I think a true sign that Froala is addressing something is one the fact that its the most downloaded WYSIWYG editor in emberaddons despite being a paid editor. And two the fact that people want to contribute issues on different platforms.
So @Panman8201 no worries dude, I'm sure no one thinks your stepping on toes your voice should be heard. It's the community that made me want to contribute.
There are a few things I really like. For example index.js, I love the separation for development and production. And the fact that you have a ALL || NONE string and array for the import. I said if nothing else that was pretty cool.
For the most part a lot of the decisions made total sense. I did struggle a bit with where a decision should be made. For example {{froala-editor options=(hash theme="royal")}} You can set option within the template or the controller. And in that case it makes total sense because its a small change, its also one of those cases where the ability to declare it in multiple locations can cause in an issue. And looks like you account for a lot these cases.
If theres is a difference in opinion it's very slight, and something that we can work through. I do believe in a single source of truth where possible. So if something should be declared one in only place to reduce redundancy over flexibility.
I debated on using the initializer for these reasons but your reasoning for using it made more sense!
I did not take plugins into account.
I'm going to take a deeper dive to see how you handles actions like:
imageManager.ImagesLoaded
popups.hide.[id] (sidenote the current plugin does not address this yet)

But overall I am in favor of much of the decisions you made, I just want to consolidate where things can be declared in only one place if possible.

[Panman82]

@artsmc No problem, certainly take some time to dig through the source. I tried to comment everything with some details (like "why do it this way").

I'm going to take a deeper dive to see how you handles actions like: imageManager.ImagesLoaded

Replace dots with dashes (rather than underscores in this addon, dashes are more "ember like"). Ex: {{froala-editor on-imageManager-imagesLoaded=(action "foobar")}}

popups.hide.[id] - (sidenote the current plugin does not address this yet)

Humm....that's an interesting one, I haven't handled that one either. Since it's not valid Handlebars syntax, it'll have to be some special usecase/microsyntax.

So no worries dude, I'm sure no one thinks your stepping on toes your voice should be heard. It's the community that made me want to contribute.

Cool, sounds like we're on the same page, it's a community effort. Which is the reason I like to submit PR's to help improve addons, it improves things for everyone else too.

[maulsea]

Sorry @stefanneculai, me & @jigneshchudasama05 have not been able to look over this addon closely after initial implementation. @Panman8201's changes look thorough & I don't think it'd be a problem to merge this in with PR. We can tag it with new major version in npm & here in repo. People can optionally grab an update (with a disclaimer that they need to change their implementation). Does that sound like a good idea to you?

[Panman82]

@maulsea I think a good approach would be to enable the ability to use all the new names in the existing version, with depreciations for current names that don't match. I could work on a PR for that. Then, pull in this new version, releasing a new major version.

Depreciate specifically;

1. {{froala-editor}} value in favor of content
2. {{froala-editor}} params in favor of options
3. {{froala-editor}} event names not using on-*
4. {{froala-editor}} customButtons in favor of using an initializer
5. index.js fontAwesome in favor of using the ember-font-awesome addon
6. index.js theme in favor of themes
7. index.js lang in favor of languages

P.S. I did try to think through everything very thoroughly, took over a month to tweak things. No hurry on this, if you need time look through the source... It's has big changes internally!

[Panman82]

FYI, I released a new alpha with support for the popups.hide.[id] event (would be on-popups-hide-id=(action) in ember-land).

And to throw out another possible path forward; transfer ownership of my ember-froala-editor repo and npm deprecate this addon. The benefit would be keeping both editions in place and folks can transition as needed. Also, I think ember-froala-editor is a more specific name (in the event that Froala releases new/additional products in the future). But I'll certainly go with the collective decision made by you folks.

[stefanneculai]

Indeed it makes sense to have it named ember-froala-editor which is more specific. @Panman8201 I added you to the Ember team. Please feel free to initialize repository transfer over the Froala organization. We'll maintain both for a while and then drop the current one.

[Panman82]

@artsmc @maulsea @jigneshchudasama05 Before I make the transfer, is everyone ok with this plan? Again, I really don't want to create any hard feelings. I'll be sure to add everyone to the repo and npm contributors.

[maulsea]

All good at my & @jigneshchudasama05
https://github.com/jigneshchudasama05's end.
We appreciate your efforts to make this addon better.

On Thu, Mar 10, 2016 at 2:14 AM, Ryan Panning [email protected]
wrote:

@artsmc https://github.com/artsmc @maulsea https://github.com/maulsea
@jigneshchudasama05 https://github.com/jigneshchudasama05 Before I make
the transfer, is everyone ok with this plan? Again, I really don't want to
create any hard feelings. I'll be sure to add everyone to the repo and npm
contributors.

—
Reply to this email directly or view it on GitHub
#18 (comment).

Maulik Bengali
+91 9820 790 760

[artsmc]

My one exemption at the moment is maintaining froala options from one location. But outside of that this is solid gold

[Panman82]

My one exemption at the moment is maintaining froala options from one location. But outside of that this is solid gold

@artsmc Can you expand on that? It's basically the same thing as the current params.

Edit: Do you mean not being able to set each option as their own attribute? Ex: {{froala-editor theme="royal"}}

[artsmc]

For example if i declare options=(hash key="foo") in the template
Then I declare options key: "bar" in the controller im introducing a conflict. Currently the template looks for the controller params as its source of truth. I was going to read through more of the source friday but having one place to make a decision where possible would safe a lot of code to account conflicts. Totally basing this off what i have seen so far without taking a deeper dive so correct me if Im wrong

[Panman82]

All options still come through the options attribute of the component though (or through the internal defaultOptions property). The (hash) helper is an Ember.js thing, and can't be limited by addons. You could do the same thing with this addon today; {{froala-editor params=(hash key="foo")}}

To expand on that, the (hash) helper won't look to the controller for string literals (quoted strings "foo"). Rather, if they didn't have quotes then it would assume to be a controller property. Ex: (hash key=foo) where foo is a property on the controller.

So basically, it's up to the developer to decide how to organize options; (hash) helper, property on the controller, a combination of both, or perhaps using the defaultOptions concept.

[artsmc]

Ok now that makes more sense. Let's go ahead and move forward with this I can help update the readme and we can write up a changelog with depreciation docs

[artsmc]

@stefanneculai do you have slack account setup for Froala I want to move the rest of conversation offline since we are all in agreement we can chat about the details

[stefanneculai]

@artsmc I added Gitter for this repo to ease communication. I am closing this issue and we shall continue to talk there.

[Panman82]

FYI, ready to transfer, just need to work with @stefanneculai to make it happen. I think temporary admin rights are needed, but I haven't transferred before so I couldn't say for sure.

[artsmc]

the one time I've done this I had the admin rights, so you might be correct

On Mon, Mar 14, 2016 at 1:45 PM, Ryan Panning [email protected]
wrote:

FYI, ready to transfer, just need to work with @stefanneculai
https://github.com/stefanneculai to make it happen. I think temporary
admin rights are needed
https://help.github.com/articles/transferring-a-repository/#transferring-from-a-user-to-an-organization,
but I haven't transferred before so I couldn't say for sure.

—
Reply to this email directly or view it on GitHub
#18 (comment).

Redberry Mobile, LLC
Mark Campbell CTO
301-323-8132
Send me a message [email protected]

[artsmc]

For example if i declare options=(hash key="foo") in the template
Then I declare options key: "bar" in the controller im introducing a conflict. Currently the template looks for the controller params as its source of truth. I was going to read through more of the source friday but having one place to make a decision where possible would safe a lot of code to account conflicts. Totally basing this off what i have seen so far without taking a deeper dive so correct me if Im wrong