Remove onclick

[cweider]

Shifts event handling into from onclick attributes into pad_editbar. Addresses concerns from #504.

See also: #529

[0ip]

now we have two .seperator's here
(l. 1527)

---

.toolbar ul.menu_right > li & .toolbar ul li have to be converted too

[cweider]

Ah, right, missed those additions – fixed.

[JohnMcLear]

Will try to test this today

[JohnMcLear]

@0ip can you test styling stuff please? Looks fine to me, some minor changes but none offend me.

I'm unable to test IE due to a script error!

Other than that this seems fine to me from quick first testing. It would be nice to test IE tho ;\

[0ip]

!? I posted a few comments on that above yours. hehe
Mobile view is quite broken. But I can "fix" that afterwards, this needs to be more dynamic anyway.

[JohnMcLear]

Okay cool, will see if @cweider can sort the IE bug too, I'd be much more comfortable pulling this after testing IE

[cweider]

@johnyma22, the IE error seems to be a part of develop… sigh. @0ip, the mobile layout should be compatible now.

[0ip]

Do we still need <a>?
Wouldn't be

<li id="bold" data-key="bold" title="Bold (ctrl-B)">
    <span class="buttonicon buttonicon-bold"></span>
</li>

sufficient?

4b272fe is broken because you shifted .toolbar ul li styling to .toolbar ul li a - that way .selected @ <li> has no effect.
Either we drop <a> or we have to add $("#" + modules[i] + "link a") in pad_editbar.js. Better solutions are welcome :)

[cweider]

This should fix it? That would be more correct since the <a> is the link. The anchor is (possibly) important for tabbing behavior, but (personally) would be nicer as a <button>. I’m actually wondering if the ul > li construction for the menu is actually appropriate now, but that’s a discussion for later…

Unrelated: the .selected styles needs looking into, it strikes me as far to general. We should probably consider increasing its specificity to .toolbar .selected or something.

[JohnMcLear]

@cweider any luck w/ the IE bug yet?

[cweider]

The resetting of document.domain added 4325f0b is what is causing the permission denied issues – @redhog.

[0ip]

Your revised version looks good to me - can you add it?

[cweider]

Added

[0ip]

Tested with IE7, works.

[JohnMcLear]

Running latest develop on beta.etherpad.org

[JohnMcLear]

http://beta.etherpad.org/p/lQa36WufrI <- spot the obvious issues w/ plugins