Remove onclick attributes

[cweider]

Shifts event handling into from onclick attributes into pad_editbar. Also has an irrelevant, but maybe nice, change from anchors to buttons in the editbar markup (IE7 looks a little funny… thoughts?). Addresses concerns from #504.

[Pita]

This changes makes it more difficult for new programmers to understand where the onclick event ends up. I also guess that this change will cause problems in different IE versions...

[JohnMcLear]

I will test this tomorrow.

[yadutaf]

Removing the "onclick" attribute sounds like a good idea. This is indeed a common practice to "bind" events to a DOM on the JS side to avoid code mixing. Then an array could hold a correspondence between the buttons ids and the corresponding action or even "guess" the corresponding action using a naming convention.

example:

• id attribute-bold => toggle bold on the selected text
• id paragraph-list-bullet => toggle bullet lists on the selected lines

But there is something I still do not understand. Why are you replacing the links by buttons ? Buttons are much harder to work with in cross-browsers applications...

[JohnMcLear]

Good work @cweider. A few Q's and pointers.

• Did we profile page load speed before/after to see if it actually made a difference?
• Buttons have lost their "spacers" IE the space between groups
• Some of the icons inside of the buttons have shifted down a few PX
• All buttons seem to function properly.
• Clear all authorship colors doesn't work in IE7/8/9 in IETester as perhaps the popup is suppressed or something, selecting a piece of text and clearing works. I'm going to assume this is an IEtester bug
• There is a new require bug in IE on the timeslider (Not replace to this pull request)

[cweider]

@jtlebi, yea, moving the bar’s construction further into JavaScript would be a good idea (see #470). Regarding <button> versus <a>, that’s mostly a stylistic thing, though it can help with accessibility a bit (tabbing works automatically).

@johnyma22, I doubt it has any effect on loading time (the <script> elements are the (minor) problem there) – just cleanup.

[JohnMcLear]

The soonest I can test and pull this is Thursday. Two reasons

a) I'm really busy at the mo
b) We're not pulling any new code until the plugin framework is finally merged

[JohnMcLear]

testing this today

[JohnMcLear]

The padding beneath the buttons is slightly too small in firefox

[JohnMcLear]

Can you please give this a quick polish then open a pull request so it goes into the develop branch? Should be good to pretty much merge once it's been polished slightly. IE seems fine

[0ip]

Update? :)

A few remarks:

• editbar buttons aren't vertically centered
• background-color: gradient... isn't valid, it has to be either background or background-image (why did you change this?)
• mobile view is a bit broken

[cweider]

This pull is invalid, at least because it’s headed into master. A version of this without converting to button tags would be better, though still may or may not makes until plugin stuff is all finished.