search

steveathon / bootstrap-wysiwyg

Tiny bootstrap-compatible WYSIWYG rich text editor
MIT License
663 stars 1.7k forks source link

Fixed #58 and Fixed #19 - throttle interfering with UI #60

Closed codewithtyler closed 9 years ago

Findus23 commented 9 years ago

I tested your fix and I am still having one issue: The active-class is only working if I change following line https://github.com/RandomlyKnighted/bootstrap-wysiwyg/blob/underscore-fix/src/bootstrap-wysiwyg.js#L98 from 

$(options.toolbarSelector,wrapper).find(toolbarBtnSelector).each(function () {

to

$(options.toolbarSelector).find(toolbarBtnSelector).each(function () {

I have only a basic knowledge of javascript, but is it possible that my HTML is malformed? Basicly it is:

<div class="btn-toolbar" data-role="editor-toolbar" data-target="#content_de">
    <a class="btn btn-default" data-edit="format-p" title="Absätze">
        <i class="fa fa-paragraph"></i>
    </a>
    ...
</div>
<div class="row">
    <div class="col-md-6 preview">
        <form method="post" enctype="multipart/form-data" id='submitForm' action="?type=page&amp;id=4">
            <div id="content_de" class="editor">
                <p>Placeholder Text</p>
            </div>
            <input type='hidden' name='content_de_submission' id='content_de_submission' />
            <div class="text-center">
                <a id="submit_editor" class="btn btn-primary btn-sm" href="#!">Saving Changes</a>
            </div>
        </form>
    </div>
    ...
</div>

bildschirmfoto vom 2015-09-21 12-28-36

Is my HTML or the Javascript wrong

codewithtyler commented 9 years ago

@Findus23 thanks for posting your issue with the following fix. I copied what your example into an HTML file and can see the issue you're referring to. Unfortunately, I won't be able to take a closer look at it until this afternoon.

In the meantime, can you test something for me? Can you open each of the example html files in your browser and tell me if those work for you? That will help me with some initial troubleshooting.

Edit: By the way, what's the HTML form for?

Findus23 commented 9 years ago

I have tested the examples and they are working correctly, except for form-post.html. (and multiple-editors.html, but this is another issue: #31). That way I have found out that the problem is Line 92 :

wrapper = $(editor).parent(),

The editor is only looking for toobar buttons in its direct parent. But in my case this is the <form>, which doesn't contain the toolbar.

By the way, what's the HTML form for? I don't exactly understand, what you mean. I am writing a CMS and sending the content of the editor to the server to process it with PHP.

codewithtyler commented 9 years ago

Ah ok. I haven't worked on any implentations of this outside of HTML and ASP.NET, so I wasn't sure.

Would this work with what you're trying to do?

    <body>
        <div class="container">
            <div class="row">
                <div class="col-md-6 preview">
                    <form method="post" enctype="multipart/form-data" id='submitForm' action="?type=page&amp;id=4">
                        <div class="btn-toolbar" data-role="editor-toolbar" data-target="#content_de">
                            <div class="btn-group">
                                <a class="btn btn-default" data-edit="format-p" title="Absätze">
                                    <i class="fa fa-paragraph"></i>
                                </a>
                            </div>
                            <div class="btn-group">
                                <a class="btn btn-default" data-edit="bold" title="Bold (Ctrl/Cmd+B)"><i class="fa fa-bold"></i></a>
                                <a class="btn btn-default" data-edit="italic" title="Italic (Ctrl/Cmd+I)"><i class="fa fa-italic"></i></a>
                                <a class="btn btn-default" data-edit="strikethrough" title="Strikethrough"><i class="fa fa-strikethrough"></i></a>
                                <a class="btn btn-default" data-edit="underline" title="Underline (Ctrl/Cmd+U)"><i class="fa fa-underline"></i></a>
                            </div>
                        </div>
                        <div id="content_de" class="lead" data-placeholder="Placeholder text">
                        </div>
                        <div class="text-center">
                            <a id="submit_editor" class="btn btn-primary btn-sm" href="#!">Saving Changes</a>
                        </div>
                    </form>
                </div>
                ...
            </div>
        </div>
        <script type='text/javascript'>$('#content_de').wysiwyg();</script>
    </body>

I moved the form opening tag up to start before the toolbar. This allows the editor to continue to see the toolbar so it can change the colors of the button. I also moved the placeholder text. The editor utilizes a special data attribute called data-placeholder in order to create the placeholder text.

Findus23 commented 9 years ago

Unfortunately this isn't helping me much. As you can see I am having two texts, but I only add the editor to one of them at a time via PHP to avoid issue #31. So it is easier for me, if I have the toolbar outside of the "row" div.

bildschirmfoto vom 2015-09-22 15-02-47 bildschirmfoto vom 2015-09-22 15-03-50

Would it be possible to add an option to use a custom wrapper class similar to https://github.com/RandomlyKnighted/bootstrap-wysiwyg/blob/master/src/bootstrap-wysiwyg.js#L323 ?

Regarding the placeholder text: This was my mistake. I didn't mean a placeholder like the HTML attribute that disappears if you start typing, but rather a prefilled editor.

codewithtyler commented 9 years ago

Ah, I misread that earlier. I thought you were dealing with a single editor. So it appears that the issue you're experience is due to the bug mentioned in issue #31. This PR is not mean to fix that bug. If you will propose your wrapper question there I'll be happy to continue our discussion and create a fix for that bug. I believe with a bit more discussion we can come up with a good solution to the problem you are experiencing.

codewithtyler commented 9 years ago

You can ignore my previous comment. I got some additional time this afternoon to look over what you mentioned. To put it simply in the following line the editor

$(options.toolbarSelector,wrapper).find(toolbarBtnSelector).each(function () {

searches all elements that matches the toolbarSelector and the wrapper criteria and applies the background color to those elements. I think the original reason this was done was to accommodate situations like yours and to also accommodate the typical situation where the toolbar is a child of the parent element. However, this was not necessary as toolbarSelector handles both situations due to the toolbar having the data-role attribute.

codewithtyler commented 9 years ago

My latest commit should fix your issue.

Findus23 commented 9 years ago

That's odd. I somehow used commit https://github.com/RandomlyKnighted/bootstrap-wysiwyg/commit/d22e3aee5c and I am now unable to find out why it doesn't appear in the list of commits (https://github.com/RandomlyKnighted/bootstrap-wysiwyg/commits/underscore-fix)

But commit https://github.com/RandomlyKnighted/bootstrap-wysiwyg/commit/1e2f216a91e71e6417600cb88aa03d91d1fc5917 is working perfectly and now only needs to be merged into the main repository.

Thanks for helping me.

codewithtyler commented 9 years ago

I merged last 3 commits together as one by rebasing them.

No problem, thanks for catching the issue.

codewithtyler commented 9 years ago

@steveathon there may be merge conflicts once you merge my other PRs. I'll be happy to rebase this once they have been merged.

steveathon commented 9 years ago

Thanks @RandomlyKnighted this one is ready to rebase now. :no_good:

codewithtyler commented 9 years ago

@steveathon give me a few mins and I'll have it fixed up for ya

codewithtyler commented 9 years ago

@steveathon alright fixed haha...I'm getting faster at rebasing now

codewithtyler commented 9 years ago

After merging this..are we ready to release?

steveathon commented 9 years ago

Yep, I believe we are. I'll do a version bump tonight.

codewithtyler commented 9 years ago

Sounds good. When do you want to do the roadmap so we have a plan for the project? I'd like to know what our goals are from here.

codewithtyler commented 9 years ago

@steveathon don't forget to release the new version