If you've ever visited a National Park or many state parks in the United States, or been involved with a nature-oriented community group, you've likely heard that saying many times. For those who haven't, the meaning should be obvious: leave the shared place that you're moving through in at least as good condition as you found it, so that the next people to travel that way can enjoy it as much as you did.That applies to environments other than the great outdoors, of course. The phrase popped into my mind earlier today as I was poking at a jQuery plugin to add a context menu to a Web app I'm developing.
It's great that there's so many free software tools out there; I've been using them for more than 25 years, and I've developed more than a few. Sometimes, however, one is reminded of the wisdom of Oliver Wendell Holmes when he said, "Learn from the mistakes of others… you can't live long enough to make them all yourself." But in the grand software tradition, we often give it our best effort.
This is a reasonably well-known plugin, one of several that offer to solve a common problem in more-or-less similar fashion. And if you use it precisely in the way that the author expected, it does the job. But even then, if you're using it in a Web app or site that customers are paying money for, you might hope that they never hit the "View Source" menu item in their browser. And, to be fair, this is not intended as a slam against Matt Kruse or his code; most of the other plugins I've looked at in the last couple of days have at least as many "quirks" and assumptions.
"It's just a context menu," I hear you say. "What could possibly go wrong?…go wrong?…go wrong?…(sound of tape snapping)" (with apologies to Westworld, a movie with many lessons on the art and craft of software development.)If you're adding the menu to your page when it's first loaded, and you want a static menu, with the options the same for all uses of the context menu, fine. If you want a more dynamic menu, and don't mind attaching functions to the menu that know enough of the detailed state of your page/app that they can generate the properly-adjusted menu items each time the menu is displayed, that works, too. But the main assumptions are that the plugin's main method will be called once and only once on a page, and that you really don't care about the markup being added to the end of your page. The first assumption, particularly in a rich front end, is likely to be naïve; the second is, bluntly, an assault on your professionalism.
What's so bad? Here's the plugin's output for a very simple menu, reformatted for clarity:
<table cellspacing="0" cellpadding="0" style="display: none;"> <tbody> <tr> <td> <div class="context-menu context-menu-theme-vista"> <div class="context-menu-item " title=""> <div class="context-menu-item-inner" style="">One</div> </div> <div class="context-menu-item " title=""> <div class="context-menu-item-inner" style="">Two</div> </div> <div class="context-menu-item " title=""> <div class="context-menu-item-inner" style="">Three</div> </div> </div> </td> </tr> </tbody> </table> <div class="context-menu-shadow" style="display: none; position: absolute; z-index: 9998; opacity: 0.5; background-color: black;"></div>
If you read HTML and CSS, you're probably nodding your head, thinking "that looks simple enough; hey, hang on a minute…" That "hang on" is where you start seeing any of several issues:
We have a table, containing a (rarely-used) tbody, containing a single tr, containing a single td, containing a series of nested divs which contain the menu items. If he'd implemented the entire menu as a table, he might have been able to claim that he was trying to support 1997-era browsers. (Would jQuery 1.4 work in Netscape Navigator 4?) Failing that, it's a mess. Nesting divs (or more semantically appropriate elements) has been a Solved Problem for a decade or so.
Even though this is implicitly intended to be fire-and-forget markup added to the DOM at page-creation time, it's disturbing that there are no identifiers for any of the table elements. If you wanted to, say, remove the table later, you'd have to use code like
$('.context-menu').parent().parent().parent().parent().remove()Ick. And then you'd have to go clean up the shadow div separately.
Speaking of the shadow div being separate from the menu-enclosing table, why not wrap both in an (identified) div, so that you (and your clients' code) can treat the menu as a single entity?
Doing that would also let you dynamically delete and replace the menu, based on changes in the state of your app, without needing to define menu-item handler functions that violate the Principle of Least Knowledge.
Another benefit of that would be if your context menu was generated more than once (possibly because the content needed changing or items needed disabling), you'd never have more than one copy of the menu in the DOM. As it is now, you can have an arbitrarily large number, with only the most recently-added being the "active" one. Ugh. This would be more likely to happen on a dev box running a test suite, but still. Ugh.
Particularly in a behaviour-driven or test-driven development (BDD or TDD) environment, being able to test/validate the markup and the item-handler logic separately as well as together is important. Doing so with this plugin (and again, to be fair, most of the others) eliminates the normal-use workflow from consideration.
One feature of this particular plugin is that it supports having you define your own HTML for the menu and pass it in. But the example given is too simple to be useful as a menu. Browsing the plugin source code seems to indicate that there is no event-handler support for menus defined in this manner; you'd have to iterate through your context menu items, assigning event handlers for at least the click event. If you're going to do all that work, why bother with this plugin?
- A table and divs. Enclosed and in parallel. (Careful wiping that green sludge off your monitor; that's your brain that just exploded.)
Menus are "important" enough that HTML 5 has its own set of elements dedicated just to marking up menus. However, sensible, reasonably semantic standard patterns for use with HTML 4 and XHTML 1.x have evolved that address the issues I've mentioned (among many others). The markup for the example menu above could have been written as:
<div class="context-menu-container" id="someId"> <ul class="context-menu context-menu-theme-vista"> <li class="context-menu-item"> <span class="context-menu-item-inner">One</span> </li> <li class="context-menu-item"> <span class="context-menu-item-inner">Two</span> </li> <li class="context-menu-item"> <span class="context-menu-item-inner">Three</span> </li> </ul> <div class="context-menu-shadow"></div> </div>
One outermost div, with an id so your plugin doesn't get confused if you have multiple menus on a page, but one div so you can work with it as a single unit. An unordered list containing list items corresponding to the menu items, since that's the closest HTML4/XHTML 1 comes to HTML5's menu semantics. The Script that builds the thing can adapt the styles and attributes at the class+id CSS level, eliminating the need for hard-coded monsterpieces such as that style attribute for the table.
If you're going to write a jQuery plugin, live in jQuery. You can dynamically modify styles, attributes, content, even whole sections of how your page is rendered in the browser, without touching the basic markup. You can put the style bits that you intend to remain constant into a CSS file and link those styles to individual DOM fragments via classes and ids. That also keeps your HTML and CSS clean and cacheable.
If we don't write clean code, then having a lightning-fast browser on a petabit-Ethernet connection won't matter; users and reviewers will still complain that "your app is slow". Think before, as, and after you code, and remember:
First, do no harm.