#13615 closed defect (bug) (fixed)
Confusing behavior adding items to nonexistent menu
Reported by: | filosofo | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Menus | Keywords: | has-patch |
Focuses: | Cc: |
Description
If you try to add menu items on the create menu panel, nothing happens:it fails silently. Then if you realize your mistake and click another tab or submit the form from the name input box, you get the page unload AYS.
Patch:
- Throws an error message when you try to add menu items to a nonexistent menu, both for js and non-js.
- Removes unload AYS when there's no menu.
Attachments (2)
Change History (15)
#2
follow-ups:
↓ 3
↓ 4
@
14 years ago
I think your reasoning about the UX is sound and agree that it's a better strategy.
As an aside, I'm really curious why you'd change the line in my patch from
if ( document.getElementById('menu-to-edit') )
to
if ( 0 != $('#menu-to-edit').length )
Would you please explain?
#3
in reply to:
↑ 2
@
14 years ago
Replying to filosofo:
Would you please explain?
Oh no, not another jQuery argument :-)
Re the opacity, Jane has said she approves this approach. I think it deserves a slight non-JS modification, in that the submit buttons are additionally disabled, otherwise clicking those would cause a page refresh and that's odd.
#4
in reply to:
↑ 2
@
14 years ago
Replying to filosofo:
As an aside, I'm really curious why you'd change the line in my patch from
if ( document.getElementById('menu-to-edit') )
to
if ( 0 != $('#menu-to-edit').length )
Would you please explain?
It's shorter, and because jQuery is WP's JS library and I like to use it wherever possible. It tends to reduce the incidence of browser-specific JS bugs.
I think it deserves a slight non-JS modification, in that the submit buttons are additionally disabled, otherwise clicking those would cause a page refresh and that's odd.
That would be a lot of code... wouldn't you have to add it to each of the buttons? I'm going to commit what I have, since what you propose could be added on.
#5
@
14 years ago
- Resolution set to fixed
- Status changed from accepted to closed
(In [15062]) Do not load window.onbeforeonload event if we are not on an existing nav menu. props filosofo. Reduce opacity on nav menu postboxes if we are not on an existing nav menu, and for JS users, deactivate all links and form elements, to make it obvious that those UI elements are not usable at this stage. fixes #13615
#6
@
14 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
We still need the non-JS version. Also, we have no opacity in IE.
#7
@
14 years ago
- Owner changed from markjaquith to nacin
- Status changed from reopened to accepted
Going to run with this.
ocean90 has pointed out that the tabs in the meta boxes also would need to get their href's stripped. I think that's excessive.
#8
@
14 years ago
Also, we have no opacity in IE.
Third rate browser, third rate results. It's a travesty that Microsoft's "latest and greatest" browser doesn't support CSS 2.1. The opacity change is a nice visual cue, but it isn't functional, so IE users can deal, or upgrade to a real browser.
ocean90 has pointed out that the tabs in the meta boxes also would need to get their href's stripped. I think that's excessive.
It's "nice to have," which is why I did it for the JS version. It makes it more obvious that it's a read-only area. Non-JS doesn't have to be as polished as JS. We want it to work. It doesn't have to work as elegantly. So don't go overboard.
#10
follow-up:
↓ 13
@
14 years ago
Replying to markjaquith:
It's shorter, and because jQuery is WP's JS library and I like to use it wherever possible. It tends to reduce the incidence of browser-specific JS bugs.
OK. The reason I ask is that retrieving an element by its id using document.getElementById
is one of the most widely-supported actions across browsers (many more than jQuery itself claims to support), and it's almost universally very fast.
I've tested it across a number of browsers, and at best (typically in the slowest browsers), jQuery is only twice as slow as the host method in retrieving an element by ID; in the fastest modern browsers it seems to be at least several times slower.
I guess I'm overdue for my promised blog post about the more general issue, but this particular line seems like a no-brainer: at the cost of a few characters you get wider support and better performance, guaranteed.
#11
@
14 years ago
Also, you can win back the characters and further improve performance if you, e.g., set d = document
and then consistently use d.getElementById
: less typing for you and less time going up the scope chain for document
.
#13
in reply to:
↑ 10
@
14 years ago
Replying to filosofo:
I've tested it across a number of browsers, and at best (typically in the slowest browsers), jQuery is only twice as slow as the host method in retrieving an element by ID; in the fastest modern browsers it seems to be at least several times slower.
I guess I'm overdue for my promised blog post about the more general issue, but this particular line seems like a no-brainer: at the cost of a few characters you get wider support and better performance, guaranteed.
I must have missed that fracas. The difference for this one selector can be measured in single digits of nanoseconds. So this one isn't going to make or break anything. I gather it's more of a general trend you're concerned about. If we want to make sure we're using the fastest way possible of doing simple DOM grabs or checks, I noticed that this is actually faster than using document.getElementById()
in latest stable Firefox:
g = function(a){return document.getElementById(a);};
For me, it consistently saved about 0.2 nanoseconds per iteration. Averaged 2.9 ns vs 3.1 ns for the POJ version. Unexpected, but then the new JS engines do some fairly insane stuff to eke out speed in function calls.
And it doesn't get much shorter than g()
:-)
If there is a separate ticket for speeding up our JS, we can continue this line of investigation there.
Nice.
We can do better. Instead of admonishing them for using the UI we presented to them, we can fade away the unusable portions and deactivate them if possible. This makes it clear what portion of the UI is meant to be used, and what part is not yet available, while still maintaining their proper spatial relationships.
Check out my patch.
Screenshot:
Jane, any UX objections?