Screen reader users and some keyboard only users need a clear, quick way to disable the overlay

Better title - I keep getting reminders in other issues that screen readers are pieces of software, not people :)

Damien Tourmond writes:

Given that everyone at this point wants Drupal 7 out of the door, my speculation is that the overlay will *never* be truly accessible. It is not as much a concern as it seems. People that want to certify sites against WCAG will be able to either (1) disable the overlay and leave it out of scope of the certification or (2) improve the overlay solution (in contrib) and disable the one provided by core.

It would seem, then, that giving the option to disable overlay during installation is really the cleanest solution. And otherwise -- hoping I'm not oversimplifying -- if we force everyone to install it, aren't we placed in the position of telling users with accessibility concerns to disable something through the very interface that is not accessible to them?

Clear, quick... can't we add a fasttoggle like menu path? I have already asked this in the per user overlay toggle patch. Don't forget that we are talking of new users. You click on a "disable overlay" link and met with a sea of checkboxes, text fields and whatnot. Awesome UX!

Hi, a first stab at the issue. Patch attached. It displays a "disable overlay"-link early in the html and exposes an additional menu path (as suggested by chx above). All the Javascript-ARIA-Stuff from #716612 has been left out. It does not address comment #4's point that the "disable overlay"-link/UI should only appear when the overlay opens; I don't have a JAWS or similar setup (nor experience :) and have only tested this in lynx and FF w/o CSS.

1) The link text should perhaps be a bit more expansive/comprehensible for new users; maybe: "If you are using a screenreader, you might encounter problems using the administrative overlays. Please click here to disable them.".

2) David_Rothstein wrote: "For someone who wants to continue using the overlay it will be annoying to have this on every single page". To address that, we could show the following text/links for screenreaders only until a choice has been made: "If you are using a screenreader, you might encounter problems using the administrative overlays. Link 1: 'It works fine for me, I want to keep the overlays' and Link 2: 'Deactivate the overlays'". Once a choice has been made these links wouldn't be displayed anymore. That is easily implemented.

Hrm, that's not CSRF protected. http://drupal.pastebin.com/0cgEAUgX

There's a patch; marking "needs review".

Ok, cleaned up overlay_user_disable() as per chx suggestions; kept feedback message and drupal_goto to user edit page / or given destination.

Outstanding points:
1) Finalise wording of the link text (and/or link title) to be more comprehensible for new users (see comment 7 above, point 1)
2) Functionality: display message to screenreaders only until a first choice (keep/disable) has been made? Or show disable link always when overlay active? (see comment 7, point 2)
3) CSS: Can #skip-link and #disable-overlay share a class? Where do we put #disable-overlay-CSS?

should be disable_overlay

we could use .invisible-element if we handle #897638: Make .element-invisible work for focusable elements like links first.

A key binding is an interesting idea. But, since this is likely going to be a one time use thing I'm not sure that it would be that benificial. Also, it is difficult to find a key binding that doesn't conflict with key bindings for the OS / Browser / Assistive technology combos that may be in use.

We're not cluttering up areas such as the installer, the user profile form, etc. for options that only affect the 0.0001% of users out there who are Drupal admins, as opposed to end users on a Drupal site. No.

@design_dolphin

I agree that the user registration form is the wrong place to put this option. It is best, usually, to place a control nearest to the feature that it will deactivate. As a simple user I would have absolutely no idea what I was choosing to enable or disable from the registration form.

Speaking as a person with a disability :)

OT:

My experience with other CMSs is limited, but I think that it is safe to say that the Drupal community has put more effort and time into accessibility than any other. Also, since my involvement with Drupal began I have not seen an issue not resolved because the quantity of users affected was marginal. We are doing a great job, the concept of placing the overlay disable link on the user registration form is just a poor design option.

Why redirect to user/X/edit?

To be clear, I wasn't pushing that the registration form would be the best option.

@webchick

Thanx for clearing that up. I appreciate it.

chx, is good with the patch if Everett is too all we got to fix is the wording "Disable administrative overlay" - is that what we use on user profile to? I feel its handy to refer in this sentence to the user profile, rather then the action?

If nobody comes up with something better:

This site presents administrative pages in a dialog (Overlay), which may not work properly with all assistive technology. Disable the Overlay from your User ProfileDo not show me this message again

The problem with the second link is that a keyboard only user may be confused. Show me what message? Since the message will likely be hidden with .element-invisible

I'd loved to reroll the patch according to #13, #28 and #31 but am still a little confused :) and have a suggestion myself, hth.

So just to restate #31 for myself: A) We display the following in an ".element-invisible"-div to logged-in users w/ overlay access permissions:
"This site presents administrative pages in a dialog (Overlay), which may not work properly with all assistive technology"
-> Disable the Overlay on your User profile (->user/{x}/edit)
-> Do not show me this message again (disables this information message and reloads the page)

B) Alternative/My suggestion: display the message above only as long as *no explicit choice* to enable (or disable) it has been made and offer following choices:
-> Disable the Overlay (->disable the overlay ->return to current page and provide "context" in the status message instead: "The overlay has been disabled. You can enable it on your user profile [linked to user/{x}/edit]")
-> Keep the Overlay (->explicitly enable the overlay -> return to current page and provide "context" in the status message: "The overlay has been enabled. You can disable it on your user profile [linked to user/{x}/edit]")

But i am happy to adjust the patch according to A) or B) :)

EDIT: Oops, didnt see #32 - I like the wording; does the link take the user profile or does it directly disable the overlay?

@32 with some tweaks

Administrative pages are currently shown in an overlay. Disable the overlay from your User Profile if you have problems accessing administrative pages. (Do not remind me again)

We still need to address how to show the message. .element-invisible is not acceptable for the links, as they are focusable. Also, the second link still doesn't make sense out of the context of the message (which is what a sighted keyboard only user would get if the links alone are shown on focus). Can we make the entire message invisible by default, but show the entire message if either link has focus using a technique similar to that used for skip links?

No, it is confusing.

It should be made clear that the overlay is an extra option/ feature, and not needed for normal administrative navigation. This until the overlay is accessible.

I like the rest of it, but maybe something like:

Administrative pages can be shown in an overlay. This overlay may not yet be accessible with all accessible technology. Disable the overlay from your User Profile if you have problems accessing the administrative pages in the overlay, in order to use the normal administration menu. (Do not remind me again)

@design_dolphin

I agree that it is necessary to be clear that no admin functionality will be lost. I don't think that we necessarily need to refer to assistive technology.

@design_dolphin

I don't really see a problem with linking to the label for the Overlay checkbox on the profile page. Users know they are going to the user profile page (the link says so). Any drawback of not starting at the top of the page is mitigated by the benefit of landing exactly where you need to be to disable the Overlay.

#41 o.k. sounds good. I can follow that train of thought.

We have two choices. 1. Provide a toggle that turns overlay off and then modifies the setting in the user profile. 2. Direct the user to their profile page to disable it them self.

1. Is more transparent for the user, and faster.

2. Gives * some * context to the user should they wish to enable again.

FYI, one of the reasons I suggested this text originally was that I think it provides both: "Disable the administrative overlay (dismiss this message). You can also change this setting later on your user account page."

You get the convenience of turning it off right away, but the info you need to know where to go to change that again later. I don't see why we need to choose between transparency and context here.

(And also, as stated above, having people who want to turn off the overlay for accessibility reasons have to go into the overlay - where the user account page is - to do it strikes me as at least a little odd....)

Lets test that with an updated patch :)

I hope it's not in too bad form to just go ahead and write a patch w/o consensus; had a few minutes at hand to work on patches right now :)

Approach:
A) Single link on top: "Click here if you have problems accessing the administrative pages (opening in Administrative overlays) or you want to dismiss this message" and linking this to B)

B) Page for more details what's happening and why; two links to 1) directly "Disable the overlay" (without *ever* going into an overlay) and 2) "Dismissing the message" (and keeping the overlays on).

Reasoning:
A) keeps the message (relatively) short and avoids another invisible link "appearing" on focus (see #35 above) B) gives more space to flesh out in detail what's going on. Overlays will be possible to disable without opening an overlay (see #45).

Texts certainly need rewriting. I hope this helps :)

I think we easily could use one user data variable to store user's overlay preference by using some constants.

If we store this value into $user->data['overlay'] we don't need $user->data['overlay_message_dismissed'].

Patch incoming in 15min or so.

Feel free to post new patches

After, but only to the parent (underlying) page. You think we need it on both? Thinking about it, you may be right then.

#54 +1
#58 +1

One thing that did come up: How does a first time user register? By looking at it, I think #54 en #58 will cover it. Anyone see any usability problems, and should the first time registration be made clearer, or is it fine like this?

I like the title, but for the description I don't understand the part 'you started from'.

How about?

'Show administrative pages on top of the page.'

or:

'Show administrative pages on top of the page you're on.'

Any thoughts?

Based on casey's patch in #54, tries to address issue raised in #58.

Attached patch:
- Uses template_preprocess_overlay() to insert the $disable_overlay_link into overlay.tpl.php (hook_page_alter wouldn't have gotten us to the 'top')
- Adjustes overlay.tpl.php
- overlay-child.css copies CSS from overlay-parent.css and adds a z-index:101 (to lay on top of e.g. #overlay-title-bar)
- Uses NULL/!isset instead of the constant OVERLAY_PREFERENCE_INITIAL introduced in #54

Can't we put this long text into the attribute title instead of the link text itself and make the link text shorter (a lot shorter)?

Here's my initial review. Note that I have tried to remain as ignorant as possible as to what's been going on in this issue, so that I can give it a totally fresh set of eyes. Apologies if any of this was already mentioned above; I'll read the issue once I'm done here.

Please remove those constants. There is no other state, so it can only ever be 0/1, and that's entirely and sufficiently communicated via 0 and 1.

This can be reverted, too - without type-agnostic comparison, the old and the new are identical (in PHP).

Does Overlay implement other modes than 'child' and 'parent'? If it does not, then we can revert to the old if/else condition.

(and elsewhere) All comments should wrap at 80 chars. See http://drupal.org/node/1354 for details.

(and elsewhere) This link text needs to be shortened to the point - to be improved suggestion:

"Click here If you cannot access the overlay. It can permanently disabled in your account settings."

Instead of suddenly using l() here, we should use the same renderable array (ideally, even through a helper function), assign it to the template variables, and use render() in the template.

To do that, FALSE has to become array(), I think.

Powered by Dreditor.

"All administrative pages open in an overlay window. Click here If you cannot access them."

After clicking, and only after clicking, we display the following message:

"Disable the overlay permanently in your <a href="@account-url">account settings</a>."

Not sure whether we have a neutral message type anymore. A green/positive status icon would look odd to me, so I'd perhaps go with the 'warning' type.

@Sun

Can you please explain how we are doing the message switch? Are you suggesting that we dynamically change the message, or that we redirect to a page with the second message?

I think that ideally there would be one message that directs the user to their edit profile page.

** that being said wording of this message is not a sticking point for me. I will give a big +1 to anything that reasonably solves the problem of:

1. Communicate that Overlay exists, what it does, and how to disable it.

2. Provides a way to get to the UI to disable Overlay.

This should cut it. If we link to the user account edit form (like the previous patch already did), then there's not even a need for a message or any kind of toggle, IMHO.

Asked for wording help http://twitter.com/chx1975/status/24496311046

So far this is the shortest, by far: You may turn off the administrative overlay and dismiss this message in you account preferences. (based on josiahritchie's , i just changed the "or" to "and")

"All administrative pages open in an overlay window. If you cannot access them, disable the administrative overlay in your account settings."

The requirements set by Everett are met?

• Communicate that overlay exists (check)
• What it does (check)
• How to disable it(check)

So RTOTHEBC!

Oh I misunderstood, there's only 1 link to show. 'You may' is unnecessary and I don't think we have to explain that the message will disappear either.

"Turn off the administrative overlay in your account preferences."

Hmm, that's rather blunt.

"You can turn off the administrative overlay in your account preferences."

Do we have to specifiy the destination of the link though? Seems more important to give a hint as to why you'd want to disable it:

"Disable the overlay if you have problems accessing administrative pages."

Seems good. Action first and enough context to decide on whether you want to.

@yoroy Hah, I think we are cross posting, last patch is ok or not?

Looks good!

I've added the isset() + TRUE condition, because I assumed that an administrator could mistakenly save a user's account, in which case the option would be unintentionally set. To account for both scenarios (no annoying link after configuration), I think we'd have to add a $user->uid == $account->uid check to overlay's hook_user_presave(), so as to not ever save that value in case it wasn't changed by the user himself/herself.

d'oh on the .element-invisible. Looks like it would be a good idea to postpone this issue on the other one then, since duplicating all the CSS that has been added previously really does not make much sense.

Powered by Dreditor.

and suggested in #49 to introduce a dedicated page which could keep the link short "Click here if you are having problems ...". The link takes you to a dedicated page with more illustrating text. On that page we show either a dedicated "disable"-link/menu-callback or a link to the user profile

+1

The link could be short and just say: "Click here if you are having problems with the administrative overlay". This could give the possibility to link to a page, and explain the situation clearly and concisely.

(It would also save time in our breaking our heads, possibly needlessly, in order to get a short link that says it all. As well as providing an accessibility page for people (with just some of the basics). Which may be a good thing for more information to be put on accessibility in Drupal in general, but I digress.)

Otherwise maybe a link?: Trouble with the overlay? Click here for alternative
or something like: Trouble with the administrative overlay? Click here for an accessible version
or something like: Accessibility trouble with the administrative overlay? Click here.

But you better make sure all the navigation is accessible then. Otherwise explain that it is a work in progress, and all input is welcomed. This has to do with expectation levels.

There are limits to how short you can make a link, and still be descriptive, but please prove me wrong. Please keep in mind that the directive is to keep links descriptive.

As I recall this is a temporary solution till the overlay is accessible, please correct me if I am wrong. So let's not spent more time on it than needed.

This topic is going a bit in circles at times. For those replying: Please keep in mind that it must remain clear that a user can use the administration pages without the overlay. A link which says to only turn off the 'administrative overlay', and giving no alternative will leave users undoubtedly confused, and put off.

Keep it rocking! =)

Edit: changed and added link text.

Please keep in mind that it must remain clear that a user can use the administration pages without the overlay. A link which says to only turn off the 'administrative overlay', and giving no alternative will leave users undoubtedly confused, and put off.

Yes, this is a good point, which the current text does not address; I think people will figure it out, but may indeed stumble on it. Perhaps could be handled just by tweaking it, though. Instead of this:

maybe something more like this:

Otherwise maybe a link?: Trouble with the overlay? Click here for alternative
or something like: Trouble with the administrative overlay? Click here for an accessible version
or something like: Accessibility trouble with the administrative overlay? Click here.

I think the problem with these goes back to the fact that a lot of people won't understand what "overlay" means.

As I recall this is a temporary solution till the overlay is accessible, please correct me if I am wrong. So let's not spent more time on it than needed.

The accessibility will certainly improve (via the other critical issue), but accessibility isn't a binary thing. At least for some older screen readers, there will still be some issues remaining that may make it difficult for some people to use. I think the unfortunate conclusion was that we'll need this message at least for Drupal 7.

+1 for David's suggestion

Administrative pages are configured to open in an overlay window. If you cannot access them, disable the administrative overlay in your account settings

Ok, here's a new patch which goes snugly with #897638: Make .element-invisible work for focusable elements like links and also uses the new text provided by @David_Rothstein.

The attached screen view is in English displayed RTL, so it looks a bit silly.

So using something like this instead:

To check the value in "case it wasn't changed by the user himself/herself."

Adjusted patch from #93 to incorporate #85.1 (altering hook_user_presave and conditions in _overlay_disable_link).

If other users' preferences shouldn't be altered by anyone but themselves, the form-element for overlay preferences should not appear for anyone but the user him/herself (or be disabled with an explanatory text), IMHO - hence the adjustments in overlay_form_user_profile_form_alter().

Re: #96 - just to clarify, comments #84.2 and #85.1 aren't 100% the same thing: there is no "dismiss"-functionality intended. All hinges on the overlay preference being set - the message disappears thereafter. Consensus seemed (to me) that having two invisible-focussable-accessible links (edit your profile AND dismiss message) is unnecessary and difficult to accomplish in a way that is accessible, "non-confusing" and efficient.

Hi Bojhan, the message is dismissed after the user chooses (i.e. saving your profile w/ "Use the overlay for administrative pages." checked) to keep the overlay activated as well as after the user chooses to disable the overlay - i.e. the message is only shown until a choice has explicitly been made (i.e. the user profile saved by the user him/herself).

Since we know that JS is available when this message appears (since the Overlay is loaded), can we use some jQuery goodness to show both links when either has focus?

The first link what we have now, the second link to dismiss the message.

I will leave this up for Everett to decide a direction on.

If we want the links only to show when JS is enabled, we'll have to move stuff to overlay.js. The issue started off in CPH with such an approach, but @seutje mentioned some issues with this approach ("it seems to cause the link to be rly late in the tab order") which drove the solution "server-side". Can have a look at that again tho ... keeping both links visible shouldn't be v. hard w/ jquery (and maybe a bit of css) ...

Won't (5) effect all mobile users without propper JS?

@bojhan

I think the answer to your question is yes. So, we need to decide if this is an acceptable trade-off.

Thanks for #97, @frega! That's exactly what I had in mind.

We have a .js-hide class, but we don't have a .js-show or .no-js-hide class.

I clearly understand why @bojhan is unhappy with the UX with just one link in #100. On the other hand, having toiled to get the JS-injection and the focus magic working (or working at least in my setup :) I have to agree w/ @sun, that the complexity and performance tradeoff is probably not negligible and maybe not worth it (albeit the sighted users will incur this slow down only until the first time they save their profile) ...

Well, new patch expands/builds on #97 but uses JS to inject links into the "parent" pages. To avoid patching the system-behaviours.css-patch (from #897638: Make .element-invisible work for focusable elements like links) in this patch, I have duplicated some css in overlay-parent.css and overlay-child.css (could be moved to system-behaviours.css in due course). You need to apply that CSS-patch. Hope this helps, otherwise jsut ignore it :)

Patch:
- Adds a second link: "Dismiss message".
- Adds new menu callback (overlay/set/%/%) - enables/disables the overlay directly; please clear menu cache
- Injects the links into the top of the page w/ JS
- Makes both links visible when one of them is focussed w/ JS

Todo:
- Decide: is this worth the hassle?
- Proper accessibility test (keyboard accessibility/tabbing works in my setup FF3.6, Chrome5 - Linux)
- If we do keep two links: Should they not read "Disable overlay, if you have problems with accessing administrative pages" (link to /overlay/set/0/...), "No, I am fine, dismiss this message" (/overlay/set/1/...)
- Provide better "URLs" - overlay/enable overlay/disable and overlay/dismiss instead of a "cryptic" /overlay/set/ and according feedback "You have now enabled the overlay" etc.

The patch in #113 already has CSRF protection.

Needs a little cleanup, I think, to put the token in a query parameter instead (that's best practice) and use drupal_valid_token(), but functionally it should work fine - that is not hard to do at all, and definitely doesn't need to wait until the D8 generalization. We have a lot of quick toggle links in core already.

As @Bojhan is pointing to, the current text has no call to action for users that find the overlay usable:

Administrative pages are configured to open in an overlay window. If you cannot access them, disable the administrative overlay in your account settings.

Instead, the only call to action is for a very small subset of users to disable the overlay. Everyone else, should they follow exactly what the message states, would never get rid of the message. That's less than ideal. I think most users are going to expect a "dismiss this message" link when confronted with a system message they can't resolve.

If it's really impossible to add the second link, the current copy needs to be reworded to something like:

Administrative pages are configured to open in an overlay window. You can confirm or disable this functionality in your account settings.

Hi Everett, thanks for summarizing the issue. My comments:

#97: falls short of your stated requisites (e.g. two links) and gives little "call to action" or guidance; but is a clean and simple patch.

#113: fullfils your requisites (2 links), but is relatively complex (still needs testing for full accessibility) and duplicates some functionality by introducing a user setting toggle menu callback for the overlay.

We could cut the injection of links via JS from #113, but without an additional menu-callback, we could not and should not have a second link to dismiss the message.

A compromise approach (incorporating #97 and short of #113):
1) Have one slightly longer link: "Administrative pages open in an overlay. If you are having problems opening these pages, want to learn more about the overlays or want to disable this message click here.".
2) The single link goes to admin/help/overlay#acessibility
3) overlay_help is expanded to include a section explaining the accessibility problems and state clearly: "If you want to dismiss the message regarding the overlay or need to disable the overlay, please go to the overlay preference in your [user profile], check or uncheck the overlay preference and press save".

The compromise avoids an additional callback and at the same time allows for a better "call to action" than a single link at the top of every page could; but no "one-click" dismissing or disabling.

Rolling a patch like that is a question of minutes - but i don't want to flood this issue w/ another alternate patch :)

Help module is optional and not guaranteed to be enabled. Instead of linking to a help page and from there to the user account edit page, why don't you simply link directly to the user account edit page, also using a fragment/jump target, and put the help text below the checkbox?

FYI: update in class name for .element-invisible (focusable) class.

http://drupal.org/node/897638#comment-3468306

Can you upload a proper .patch?

Thanks!

"Display Overlay preferences only if the current user can access the overlay and when a user edits the own profile."

The faster condition should always come first; i.e., let's flip the conditions.

btw, this fieldset violates UX guidelines - there's only one form element in it.

I think we should always display this help in the regular description.

1) No paragraphs.

2) We can skip the "Accessibility note" prefix.

3) When appending to the regular description, this can be shortened:

"Disable this option, if you have problems accessing the overlay. Save your profile to dismiss the accessibility message shown at the top of every page."

"Prevent someone else from setting the overlay preference for a user."

Powered by Dreditor.

"Disable this option, if you have problems accessing the overlay. Save your profile to dismiss the accessibility message shown at the top of every page."

Confusing, since most people will never see the message.

Let me try to move this issue a bit, I think we are very close - and we shouldn't worry to much about copy writing. My suggestions

Top title "All administrative pages open in an overlay window. If you cannot access them or want to dismiss this message, see your account settings."

Option description "Disable this option, if you have problems accessing the overlay. Save your profile to dismiss the message shown at the top of every page."

Lets recognize this though :

• We have two actions in 1 link = usability issue
• You have to save an unchanged form to disable a message = usability issue
• The overlay has a wierd presense on the account settings (one fieldset)

Fairly sure we can't fix all those issues, especially the wierd workflow we introduce by having one link just sucks. But we have to get this solved soon and maybe these trade offs are oke? Anyway, I only really wanted to comment on the copywriting.

I understand that "Overlay" is a druplism, but quite honestly "a superimposed window" sounds horrible and I do not understand why you see it more comprehensible.

The overlay is a concept relatively new on the web, unless we represent it as some kind of giant modal dialog I do not see us coming to an easy solution on this one. Lets not forget the bigger picture, its to get people to simply move on - administrative pages that open in "something" that you can disable if it's not accessible. Untill we test it, it will be hard to know what the word overlay conceives, going with "Overlay" is somewhat oke - as long as its preseeded by administrative pages open in ...

Hah, darn issue queue communication. Oke,

If were are so unhappy with the placement of the overlay preferences on the account settings page (@bojhan: "weird placement", @sun: "violates UX guidelines"), can we move it a separate tab (MENU_LOCAL_TASK) like e.g. the shortcut-module?

This would certainly give us more freedom with the form ... i'll roll a small patch if that seems reasonable and productive to anyone but me :)

Yeah I definitely think saving an unchanged form in order to make a message (that you can't see..?) go away is very very strange. Especially because a user could easily save their profile for a completely different reason altogether (change their email, contact settings, etc), and that action would disable the message unintentionally. Maybe make it radios..?

* Display administrative pages in an Overlay window
* Disable administrative Overlay windows

And have neither selected by default..?

Also, I realize that the lingo has been a bikeshed-fest, but I'd like to just point out that specifically mentioning accessibility concerns in any of the messages isn't necessary is it? Plenty of people will want to disable them even if they're not experiencing accessibility problems.

edit: Also, there's a typo in the last patch:

I think we are all waiting for a patch, mine or jeffs suggestions in #142 will be fine!

1. I would agree that two links are better than one, but I can go either way.

2. I am quite happy with everything that David has suggested above.

3. #897638: Make .element-invisible work for focusable elements like links has been committed in case anyone didn't notice.

In the last patch (#131) when I check the option to use the Overlay I am not redirected back into the Overlay when I save my profile. I have to go back to another non-overlay page to re-enter it.

This is assuming I have unchecked the option, then changed my mind and want to re-enable it.

In agreement with David. We need to come to consensus on the solution. I was hoping that my issue summary in #118 would help us get there, but it has only in part.

I have spoken to Everett about this and I think his sum up is pretty complete. I hope this will bring the discussion out of its stall, its mostly a technical discussion really.

@Jeff

Nothing show stopping about making the disable link a callback. However, it is nice to give the context of sending the user to the profile page so that they know where the option is should they wish to re-enable.

As far as performance, it is a hit, not sure how much. I'm not too particular, I suppose they cold be in parent and child tpl.php files.

One note I forget to add above, and I will add here for completeness is that the links should appear at the top of bothe the parent and child frames, so that it is more likely that they will be found.

I agree with Everett, its not fool-proof enough, even if the user does not have access to the Overlay initially, they might escalate their privileges later on and acquire access, in which case they are immediately stuck.

Everett, you're right: unblocking a (new) user (with 'access overlay' permission) has the user save the profile (implicitly the overlay preferences); this would dismiss the message before the user properly realises what's up.

Rerolled the patch using a separate flag and a dismiss callback. #150 should now be covered. Markup and wording for "invisible heading" still needs work. Includes applied patch from 716612 as advised in #158.

Overall not very happy with this: a) another flag and b) the accessibility message adds but for most users this message will be "unknownable" (if relatively light) bloat on admin pages; I added a dismiss-link to the checkbox description on the profile for sighted users (but who'll use that).

Maybe this calls for a different approach - will find you/bojhan in IRC to discuss it :)

rerolled #159 (to heed #158, sorry).

Rerolled patch is attached, with the changes listed above, as well as some smaller ones (mostly code style and small bugfixes and that kind of thing). Among the other changes worth mentioning:

1. The message that you see after you click the "dismiss" link previously said:
   

   The accessibility message for the administrative Overlay has been dismissed.

   I changed it to this (to make it more direct and actionable and try to remove technical language):

   The message has been dismissed. You can change your overlay settings at any time by visiting your profile page.

2. I also removed all capitalization of "Overlay" from the patch. (If we want to change that, we'd need to do it consistently everywhere in the module, which really sounds like a different issue.)

STILL NOT DONE:

1. The visual styling of the message is horrible (and close to unreadable for keyboard users). I am not sure what to do about that; it really seems like we want this to have similar styling as the "Skip link", though.
2. If you click on the "dismiss" link from inside the overlay you go through a couple weird redirects and wind up on the same page but outside the overlay, which is pretty odd.
3. I think this text still needs some work:
   

   Disable the overlay on your profile page if you have problems accessing administrative pages.

   It does not give a clue about what the overlay is; I think we need to borrow a little text from the user account page to explain that, or no one will understand this message.

   I'm also not sure if it should say "profile page" here vs "account page" - we actually aren't consistent with that throughout Drupal, but "account" sounds better to me personally. This is less important.

Please avoid working on 3, for now - lets focus on getting all those technicalities out of the way.

@jeff Why not go with yoroy's suggestion of using the back to blocks page styling, rather than creating a new one?

@Jeff

Can you please provide a description of overlay-disable-message.png?

Thanks

@Everett:

The links are contained in a box which is very dark gray, which contrasts with both the overlay modal background (semi transparent black) and the overlay page (which is white) - it needs to do this because it can overlap both. The box is centered and has width of 36em, so it can grow if the text size is increased.

There is no border or any other "width adding" styles applied to this box, we can't add any width or height to it or it will show all the time. Instead the links have some padding and margin.

The links are white, to contrast with the dark gray of the containing box and are underlined on focus. Both links are set to display block, so they are not inline, the Dismiss message is therefor on a new line at the bottom.

Just read the thread, plus the ancestor. Quick summary to save others the time: summary in #118, agreed approach in #150, active (and nearly there) patch in #163. 3 outstanding issues listed in #163.

I have tested the patch:
- It works.
- It matches all the requirements in #150 except #150.4 (see below).
- The workflow is not too annoying, although I haven't tested it with a screenreader.
- Only problem is 163.2: we fall out of the overlay after dismissing the message.

Outstanding issues:

163.1 - visual styling. no comment.

163.2 - odd redirects after you click to dismiss the message. This is what happens:
- Dismiss link goes to http://example/overlay/dismiss-message?destination=admin&token=blahblah
- Redirects to http://example/#overlay=admin (with the message saying "You can change your overlay settings at any time by visiting your profile page.") but outside the overlay.
- Redirects to http://example/admin (without the message) again outside the overlay.

This needs to stay within the overlay after the message is dismissed, and the second redirect needs to be stopped.

163.3 - text needs some work. 150.4 stipulated 'Links to be preceeded by invisible heading "Options for administrative Overlay", we can work on copy.' I think this would help. Specifically, it should say: "Accessibility Options..." or "Screenreader options..." The current text, without the accessibility context, makes it sound like there's a problem with drupal/that we've shipped something buggy (I accept there's an argument there, but let's not have it in the UI).

[EDITED to correct what I said about 163.2).

163.2 - redirects.

AFAICT, redirecting the entire page from the overlay (as now) won't do, because it loses anything the user may be doing in the underlying page, which is hard to justify. I'm not sure if it's possible to do the redirects in the overlay but the most elegant solution from the user perspective is probably to call overlay_user_dismiss_message() via AJAX, like overlay_ajax_render_region().

...which I don't have time for right now, sorry.

There's also a grammatical error here, ending the sentence with a preposition. Should be "Show administrative pages on top of the page from which you started." although I admit that might sound confusing.

That's my only input, I can't actually test the patch ATM. I just wanted to throw this out there again, because while Overlay accessibility is a big part of all this, this particular issue isn't necessarily specific to that user-group. Screen-readers need a direct correlation between the Overlay itself and where to disable it yes, but ALL USERS need a clear way to disable the Overlay.

@David_Rothstein: Sorry, I didn't mean to imply that you had added that extra sentence to the description, in fact I was aware that it had been added much earlier. Glad we're in agreement on that point though, I had mentioned it earlier in the issue but it got glossed over.

As for the grammar, I'll cede to you on that point though I didn't see anything in particular when browsing through some of the results of that Google search. I'll leave it alone though, as this issue has been about verbage for far too long. :-P

Oh, and another thing: I made the screencast using Chrome. I tried Firefox but tabbing there never got me to show the skip-link nor the overlay message. Instead, the search box in the underlying Bartik page gets focus. This is on a Mac, would be good for someone to double check this: when in overlay, hit the tab key. do you get focus on elements in the overlay or on elements in the underlying page?

@roper:

Sorry, I didn't mean to imply that you had added that extra sentence to the description, in fact I was aware that it had been added much earlier.

Oh, sure, I understood that - I was just pointing out that not only did I not do it, I don't know why it was done :) Whoever did come up with it should probably speak up and explain or we will just take it out...

@yoroy:

- Check the box to start using the overlay again and save.
- We stay on the profile page, which is ok
- But now: let's directly click a toolbar link for an admin page
- None of them opens in the overlay

That's actually a separate bug (it exists without the patch too) - can you make a separate issue for it and link to it from here? I think I might know what's going on and have a theory for how to fix it, but better to handle separately since we have enough going on over here :)

I am very wary of this direction, it is very intrusive and even more so with this patch. Is this really what we want? To me this message should not render the underlaying contents unusable - which the proposed does, this because its overlaying the actual content. Another point is that the styling and shape of this element is not found anywhere else in core, the blue color is - but using a "modal" with a bottom bar for dismissal is not a common element.

I would highly favor if we go for something far less intrusive, for example the "back to blocks admin" styling and shape we have on the blocks demo page. I see the patch already tries to use the blue, but the position and shape is the most important part.

A visualization to my idea

Working on a new design patch now - #187 is mostly doable but I need to know if we want this to push the content down or live in its own layer, I think it actually looks better if the content is pushed down. The radius styles are not doable because the dismiss message can wrap when the browser width is narrow (also when the font size is very large).

IE7 nightmare report - this is a separate issue altogether - right now when you start tabbing the underlying page gets focus in IE7 (maybe IE8 also, have not tested), this is very serious issue because if you "click" (hit enter) on the skip link you will be taken to the main-content of the underlying page not the overlay page. This is an extremely bad ass situation and can someone else confirm/deny, would be so good if this is an artifact of my system.

Adding two screenshots to show it on one line and wrapping, + a pretty rough patch just for testing. I have removed the full stop on the dismiss message in these screenshots and my font-size is quite large (text re-sized up a few notches in my browser).

Note: this question of whether to push the content down or not is curcial to how this can be styled, its gets somewhat harder to achieve the "all on one line and centered" style if we want it to overlay the page.

yoroy - found this issue, seems to be appropriate: #841184: "Skip to main content" link doesn't work correctly in the overlay

Am I right to assume this is totally blocked by #841184 ?

Let's continue work here if possible. The skip link problem is nontrivial -- I've spent several hours on it and gotten nowhere -- but it absolutely has to be fixed. And whatever we come up with for that issue should fix the ordering problem here too. So let's get the toggle links working and styled correctly, and worry about tab order in the other issue.

The #182 CSS is a lot less fragile than later iterations because it does not rely on being positioned as accurately, its just "near the top" so to speak. Later styles rely on being positioned near pixel perfect to pull off the rounded corner "reverse tab" look, and this could be fragile.

Isn't the whole point to be intrusive at this juncture in a users journey? Whats your proposal, that we be diminutive and hope they don't miss it :P Personally I am not a huge fan of the dialog look an dprefer the reverse tab, it stands out a hell of lot in any regard.

What about pushing content down vs overlaying the content. To me it makes more sense to push it down, this feels more clean and does not obscure anything.

Try the patch in #189, that should give you pretty much what you are looking at in 189 screenshots - instead of overlaying the overlay like a dialog would, the message will take up space and thus push the entire page content down to make room for itself.

Why would you not try the last patch?

/me shuts up

On wording, OK with David Rothstein's wording in #176, except flip the "if" statement in front in the first bullet:

This site displays administrative pages on top of the page you started from.

-If you have problems accessing administrative pages, [link]disable the overlay[/link] on your profile page.
-[link]Dismiss this message[/link].

Trust me — it's clearer with the "if" before the "do." If you need more background, look for Ginny Redish's book on Writing for the Web.

And a preposition is a fine word to end a sentence with. If you think otherwise, look for and read through John McIntyre's blog for many, many screeds against that deeply ingrained grammatical falsehood.

Score 1: it installs against HEAD.

On IE6, Win XP: register a new user (and new users having the right permissions to see the overlay and toolbar).

- The message appears when I tab

Good news: dismiss works—
- Go to the edit my account page in main window, and change the email address slightly (but without saving)
- Click configuration on the toolbar and that loads in the overlay
- Tab through to dismiss this message
- Something happens (screen flickers, possibly reloaded) and I'm back on the overlay on the config screen, with the message saying it's been dismissed and I can change settings on the overlay page
- The message no longer appears on tabbing through
- Close the configuration page on the overlay and I'm taken back to the view my account page. The edit page is closed and the half-done change are gone. But this is what happens even if I just load the overlay and then close it again, so isn't a bug with this patch.

Good news: disable the overlay works—
- Go to the edit my account page in main window, and change the email address slightly (but without saving)
- Click configuration on the toolbar and that loads in the overlay
- Tab through to 'Disable the overlay...' and press enter
- Taken in the main window to my edit account screen. This is reloaded so that my unsaved change is lost.
- Untick the box and press save.
- Returned to the configuration page outside the overlay. There is a message saying "The changes have been saved."
- Click Home
- Click Configuration on the toolbar, and it loads outside the overlay.

Bad news: turning the overlay back on doesn't have immediate effect—
- After switching off, go into account | edit
- Select use the overlay, save
- Message saying saved
- Click configuation, opens in main window
- Click a few others, stays in main
- Click home, click account | edit, opens in overlay (I could swear it didn't used to, as the notes above say*)
- Close that and configuration opens in the overlay

* So I tried a fresh install with the patch and edit account opened in the overlay. Human error, probably?

Other questions:
1- When you are redirected to the edit account page to unclick the overlay box, whatever the underlying page was is lost. Does that matter? It would annoy me if I were, for example, halfway through editing a node. It's not critical because it's so unusual but it's certainly inelegant.
2- Small thing, but might it be a good idea to modify the account editing page to add a more specific message when it's saved so that the overlay is being disabled, saying that administration pages will no longer appear in the main window?

Other than fixing it so that turning the overlay back on works immediately I think this functionality is ok but it would be good if someone else could think about those two questions.

I haven't looked at the code at all yet.

People are welcome to create users at http://overlay2.fullfact.org/ to test for themselves. Would be good to follow the steps above in other browsers and esp. with a screen reader for usability.

Bad news: turning the overlay back on doesn't have immediate effect—
- After switching off, go into account | edit
- Select use the overlay, save
- Message saying saved
- Click configuation, opens in main window
- Click a few others, stays in main

That was already discussed (see @yoroy's reviews above) and is a separate issue at #931730: Enabling 'use overlay' setting on profile page does not immediately use overlay for admin pages

I just spent a couple hours reading some of this issue's history, and referenced issues. I will spend some time next week trying to help keep this moving.

@David_Rothstein: since #176 is your patch, and from what I can tell, the latest functional one, do you want to re-roll it to include feedback since then, or do you feel that's still the important one to be reviewing?

I think Jeff and me agreed upon #189 just need to clean up some bugs that accompany such style.

Thanks. I'll re-roll #213 with #189, and post a new summary of the issue (building on the excellent summaries already here, like #150).

Question: Why is it necessary/desired for the message/link to become visible on focus? As I understand the issue, the message/link is for screen reader users using legacy/lame screen readers that are unable to handle any kind of modal dialog. Does the message benefit anyone who is able to see the screen? Sure, people who can see might want to disable the overlay just because they don't like it, and they can do that by disabling the module or by going to their account profile. That's not what this issue is about though. This issue is about addressing the problem that people using some screen readers cannot do that, since both of those steps involve using the overlay by default, so it's a catch-22. The message/link is there to:

1. Notify someone that there might be content that's not being read to them.
2. Give someone a way to edit their account settings outside of the overlay.

So, again, this message must be audible, but why does it ever need to become visible? Just wondering what the motivation is for figuring out how to style it.

@effulgentsia

The only thing I would add to David's explanation is that there * may * be some keyboard oly users who for some reason (user agent) might not be able to properly use Overlay. This will provided the added benefit to them of an easy path to disable for their account.

I can't necessarily point to users or UAs that might result in this being a problem, but since there has been practically no usability testing of this UI component with keyboard only users it is likely better to err on the side of caution.

Overall, I think the patch has come a long way. It feels like we're getting close to consensus, and a genuine solution to the accessibility problem without sacrificing usability. Here's some problems I found in my testing:

• If "access overlay" permission is available to the anonymous role, then the links are still shown, but they don't make any sense, because they try to update the user record. Is it the case that we need to force "access overlay" to be disabled for anonymous, or do we want an alternate link that disables the overlay for the session only?
• If the user has JS disabled, the links are still there and focusable (and I'm guessing a screen reader would read them). But if JS is disabled, there won't be an overlay. So, should we add a display:none for no js? If so, what's the best practice way of doing that? We have a html.js selector, but no html.no-js selector.

Also:

I don't understand the code comment. What's an "accessibility wrapper" and why do I want to show all focusable links in it? Perhaps something like: "There are two links within the message that informs people about the overlay and how to disable it. Make sure both links are visible when either one has focus."

See above.

Finally, the changes to Seven's style.css look pretty extensive. But all site pages (including the home page) can be an overlay parent. So that means Bartik, Garland, and all contrib themes are gonna have to do something similar? Yikes! See also #639460: Evaluate CSS of #skip-link, .element-focusable, and upcoming "disable overlay" links for their impact on contrib/custom themes, where I'm worried about that same problem existing for the "Skip to main content" link. Is there any way to put more into core CSS files, and require less theme-specific rules to make a "visible on focus" link visible and legible?

This includes code to address my concerns from #220 regarding anonymous and no-js access. I don't know if the CSS code added to overlay-parent.css is any good, so please feel free to change that.

Regarding the last paragraph in #220, I escalated #639460: Evaluate CSS of #skip-link, .element-focusable, and upcoming "disable overlay" links for their impact on contrib/custom themes to critical.

See #213 about remaining todos regarding bike-shedding the text. On the one hand, perhaps that's not critical. On the other hand, we need to freeze strings in RC1, so maybe it is.

One minor suggested improvement to the text (despite #223):

If you have problems accessing administrative pages on this site, disable the overlay from your profile page.

The previous text was ambiguous in that it could be interpreted that the overlay was "on your profile page" when in fact it was the control to disable it that was on that page... Using "from" instead of "on" eliminates this minor potential confusion and does not lose any of the previously intended meaning.

Effulgentsia in #222 asks for someone else to improve the CSS part of the patch: needs work for that

Nice CSS simplification was achieved for #skip-link in #639460-60: Evaluate CSS of #skip-link, .element-focusable, and upcoming "disable overlay" links for their impact on contrib/custom themes. It would be awesome if someone could see whether something similar could be done for the CSS in this patch, as well as also implement the necessary CSS for Bartik and Garland (#222 contains only CSS for Seven, but all themes can contain the new links added by this issue). Again, we need to make sure to not overwhelm contrib theme developers with an "OMG: I need to implement several dozen lines of complicated CSS, or else my theme isn't accessible" first impression of Drupal 7. To the extent that theme CSS in #222 is just about making it look good, we're fine. But to the extent there's rules that are required for it to be accessible, let's make sure we move those to system files.

Also #841184: "Skip to main content" link doesn't work correctly in the overlay is now RTBC, so we're really quite close on this.

I tried pulling the CSS out of Seven and into overlay-parent.css, and I was totally unsuccessful. So we could use a CSS wiz here.

James, don't we need to style the hide overlay link on the parent page as well? (Of course, that one can be below the toolbar... blech.)

Here is my patch. I think I've addressed all the remaining issues. Any concerns about the specifics to how I handled the core themes should probably be filed as bugs against those themes and outside the scope of this critical issue.

1. I solved the issue in #225 where the -20px top margin was pulling the page content up
2. I moved the layout styling from Seven's style.css to overlay-child.css like ksenzee was trying to do in #230
3. I checked on ksenzee's comment in #232 and found that no hide overlay link is added to the parent frame, so there was no need to style it.
4. I added default styling for each of the core themes for the disable overlay link. I tried to match the styling of the skip to content link in each theme. For Garland I matched the styling of the RTBC patch here http://drupal.org/node/639460#comment-3640256.
5. For Jeff Burnz concerns in #235, I've only added layout based CSS properties to overlay-child.css. The global CSS has the bare minimum of styles in it. Color, borders, border radius, and backgrounds are all defined in the themes.

I've not done any cross browser testing other than Chrome, FF 3.6, and IE9 that I always have open on my dev system. So testing is still required.

This is my DOM structure on the home page

http://www.screencast.com/users/james.elliott/folders/Jing/media/906dcb3...

There is no disable overlay link there. Am I trying to work off an outdated patch?

I would love to get Jacine's input on whether there should be some default styling in core. I tend to agree with Jeff that this should be styled by default. If you don't use the keyboard you might not even know these links exist, let alone be moved to style them. But since we're trying to do as little default styling in core as possible, it would be great to hear from more themers.

Okay, you asked :)

I was expecting to actually see a message, not for it to be hidden. Once I saw it, I was surprised because it looks really messy in general. I'm really not a fan off this as hidden text. We are literally scattering easter eggs throughout Drupal. I don't understand why we are going out of our way to hide this, instead of just making it a regular message...

Agreed. If you have CSS enabled and are a keyboard only user you might not find this message. if you don't use the keyboard you almost certainly will not see the message (not that this is really that big of a problem).

Don't think it makes a lot of difference in this situation to use keyword font sizing or ems, keyword has the distinct advantage of locking the relativity to the browser default as opposed to relying on the theme.

I like consistency and this isn't inconsistent. The patch also sets "small" in some themes and ems in another. I don't really see why relativity to the browser default is more important in this case than with anything else, but it's a pretty minor issue in the scheme of things, so I'll stop complaining about it. :)

So... I think most of us agree that default styling should exist for this, so I suggest we take the "Stark" approach first and have the themes adapt from there.

'#prefix' => '<div id="overlay-disable-message"><h3 class="element-invisible">' . t('Options for the administrative overlay') . '</h3>',

Not sure why this isn't in a theme function, but can we get a class to style against for this that tells us it's focused? We can't easily style the container, i.e. adding padding or borders, without introducing visible issues otherwise. I tried adding a class in the code below, but it doesn't work inside the overlay:

Any ideas? :(

Also, this is not desirable:

'#title' => ' ' . t('Dismiss this message.'),

If you choose to style the links with text-decoration: underline, the space looks abnormal (see screenshot). If the space is removed the links are just on top of each other. That space should be added to #prefix, so this doesn't happen:

Can someone summarize why showing this would be preferable and why showing this would be helpful, I am not clear why this would be the case. I can see this scenario - keyboard user is having many troubles (say IE7 user), they can see this message so keep tabbing in order to get to it, which will happen eventually?

I think hiding this message for mouse-only users - and possibly showing it on touch-screen devices, as Jacine suggests - is going to serve everyone well, no matter what their circumstances. I have tried and tried to think of accessibility concerns we haven't addressed that will keep sighted admins from using the overlay easily, and outside the touch-screen issue, I can't come up with a single one. Cognitive impairments, limited mobility, color blindness, seizures, low-literacy sighted screenreader users, attention deficit, low-vision high-contrast environments, keyboard-only, mouse-only, mouse+keyboard combination -- none of it should be a problem. If you can see the screen at all, and you're using a browser from anytime this century, you will be fine. Either the overlay will work for you, or it will be patently obvious how to turn it off. For example, some keyboard users will find it inconvenient to use the overlay with IE6 and IE7. Not impossible, mind you; just inconvenient. Those users will see this message loud and clear at the top of every single page.

So I beg you all, please, pretty please, let's not revisit the question of whether these toggle links should be shown to mouse users. We're almost done with a patch that I believe will serve everyone's needs. And when it's done, we'll have finished every single critical issue, and we can release this amazing piece of software.

I agree with #244 quite strongly, I also have thought long and hard about this which is why I asked for good arguments as to why this is being raised again (showing the message by default). I can't see any real benefit, on the other hand showing this would add cognitive load to all users and be one more thing in the way of getting on with using Drupal. So I would ask also, no bikeshed on this please, lets get on with it and IF over the course of D7 this becomes an issue for some unknown/unforeseen reason then we revisit it in D8, otherwise this is looking good to go bar the shouting over style.

@Jacine - I just now realized why I used small font keyword (I was racking my brain as to why I would not just use ems), it was because the block region demo page link uses small, and I largely cut/paste the styles when I was prototyping.

@240 - this would be hugely helpful for styling these messages, any takers?

Ok, Here's a patch and a gazillion screenshots.

I took direction from the screenshots posted above for the default styling. The structure is pretty much the same inside and outside the overlay. The main difference is that inside the overlay it's constrained to 80%. I added wrapper divs to the markup and styled it like a 2-column layout (80% for the profile link, 20% for the dismiss link). Since it's fluid, it works nicely when the window is resized and should work fine for translation. I took screenshots of most browsers where it's wrapping to demonstrate this. In case it's not obvious, when the browser is wider, no wrapping occurs. I also added a class to the wrapper in the JS when links are focused indicating that, because it was need in order to add certain styles, like padding, to the links inside without affecting the page while hidden.

Semi-unrelated, but fixed in this patch, is the following:

Other oddities, unrelated to this patch, but worth noting for testers:

- In Seven, in IE6 and IE7, the #skip-link div is causing an extra space at the top, so in the screenshots it's not flushed with the toolbar.
- In Garland (when it's set to the admin and user-facing theme), in IE6, I cannot get the overlay to load.

After having done this, and seeing what these colors look like with all the themes, I'm thinking it's probably be best to leave those colors to the themes to deal with entirely. As of right now, all the non-structrual styles are in system.theme.css. I'm not too keen on them being in there, but I suspect that they will end up in themes, and it beats having to put them in overlay-parent.css and overlay-child.css twice, or having to create another CSS file just for this. This is less of a big deal ATM, those can always be moved around and the theme-specific styling can be done in follow-up issues, as long as we are all good with the rest of it.

Please review :)

Well this is pretty big, so I think it will take a few reviewers. I can say that the patch applies cleanly for me on a fresh D7-dev and that I get the behavior of providing the link to disable overlay, as well as the link to dismiss the message, both in hidden divs. I get the divs as expected on Chrome 7.0.517.41 Beta and Firefox 3.6 on Ubuntu 9.x in Bartik and Seven and they both appear as would be expected when tabbing. Patch also runs cleanly through Coder.

Hopefully, I can do some more thorough reviewing tomorrow or others can come in and add their voice. I do agree that we shouldn't bikeshed this issue, and the current behavior is very good.

Just gave a quick look to the patch and noticed some trailing whitespaces.

Powered by Dreditor.

Having themes style this message is rather inconsistent with other messages in core - all warning, error, OK, status reports, block demo link etc etc have core styling and do not rely on the theme. This enforces a consistent UX and frees themers from the tyranny of having to account for back-end stuff. I'm not entirely thrilled we now have to special case for this one system message.

From a usability standpoint I am interested to hear the thinking behind showing these messages outside of the overlay when no overlay page is loaded - this seems rather odd because if I have not used the admin section / overlay then I have no real idea what this means. I did test this on some willing dinner party guests and mostly they shrugged shoulders and moved on (they were able to "muddle through") so maybe not a big deal?

Hitting "Dismiss this message" gave me a 404 error (Seven as admin, Garland as front end theme) - because of this I can't really comment about the "button mental model" concerns, although theoretically I would tend to agree with Bojhan.

Bartik appears to be hiding the skip link in Overlay causing all sorts of hilarity, need to look at that more and open a new issue for Bartik.

I think it is important to have the message at the top of the DOM and of the Overlay segment of the DOM so that when a UA / AT doesn't properly follow focus the message is still reasonably (?) easy to discover.

Yes, that is what I thought, but only when an overlay page is actually loaded, not all the time.

Having themes style this message is rather inconsistent with other messages in core - all warning, error, OK, status reports, block demo link etc etc have core styling and do not rely on the theme. This enforces a consistent UX and frees themers from the tyranny of having to account for back-end stuff. I'm not entirely thrilled we now have to special case for this one system message.

This is not like the other core messages:

• It's a one shot deal. It's entirely static, doesn't need to hold multiple messages.
• It doesn't use drupal_set_message(). Applying .messages would likely introduce unwanted margins and padding, icons, etc.
• It is not remotely in the same location as the ones that use drupal_set_message(). It's not unreasonable to assume the .message styling totally depends on it's container which is totally different in this case.

My main concern is throwing in all sorts of colors on this thing, that will not usually match, and that most themes will need to worry about undoing. It's very clear to me that the proposed design does not look good in all the core themes, and what happens when someone does a theme that has a scheme with deep reds or blue colors? Do you think this pastel blue color is going to work? The worst part is that most themers will never even figure out that it exists.

I'm fine with the way it is in Stark, with this patch, TBH. It has structure and it will inherit my theme's font and link colors. I think that is just fine. No need for my themes to special-case anything. IMO this really does not need to be a Christmas tree with all sorts of flaming colors. It's really not that complicated.

If you don't like what I'm suggesting, can you please suggest something else that takes the concerns I've raised into account? I was asked to come here and provide my input and try to help, and I'm trying, but I really don't know where to go from here.

@258/@259 ok I'm not going to argue here for the sake of moving on. The style does need work to align things better.

If we can remove the message from the DOM when outside the overlay with display:none; that would at least removes the wtf. I assume we need a little JS magic to insert a class.

Right now the biggest problem seems to be the 404 for the "Dismiss this message" link.

This patch adds a theme function to separate the functionality of overlay_dismiss_message() from its markup. Because of this, it also removes the leading "_" from that function name.

It also changes the CSS in overlay-parent.css to use html.overlay-open instead of html.js to undo the "display:none". This also removes the need for the inline comments, since it's now obvious what the purpose of that is. This should be tested with screen readers, IE6, IE7, and modern browsers to see if it has the desired effect of hiding the parent document's message until the overlay is in-use, while still making it accessible when the overlay is in use (accessible both to screen readers and to keyboard tabbing).

Right now the biggest problem seems to be the 404 for the "Dismiss this message" link.

I couldn't reproduce that. I think you just need to empty your Drupal cache after applying the patch.

Also, relying on the "button" styling is a little bit of a hack (is class="button" ever even output by a theme function in core??) but I guess if core themes already have styling for it they are already assuming it as a convention for whatever reason.

The setupDrawer() function in dashboard.js adds a "button" class to a link. Therefore, I see no problem in theme_overlay_disable_message() doing so. If we think it's good UI for the link to look like a button, I think this is the correct way to do it. I leave it to others to decide if this is good UI, but it makes sense to me, since it is an action link rather than a resource link.

I also cannot reproduce the 404 on "Dismiss this message" claimed in #254, on both patches in #248 and #262.

@Jeff: Can you give the steps to reproduce? (After clearing your cache)

If we can't reproduce the 404, can we get some consensus on what needs to happen for a pass here? For what it's worth, the patch applied cleanly for me, but I'm not sure from the above what needs to be accomplished concerning theming/UI/design before this gets a pass.

Copying Everett's statement from #255, that is I think relevant to #265, I'm just not sure how:

I think it is important to have the message at the top of the DOM and of the Overlay segment of the DOM so that when a UA / AT doesn't properly follow focus the message is still reasonably (?) easy to discover.

Ok, that was totally weird, and scary. I didn't submit that comment. I was only previewing. :s

Anyway, the patch does not remove the styling from overlay-parent.css as I mentioned in the first bullet. I'm still not clear whether that needs to exist or not based on @effulgentsia's summary in #265 and #266.

So, the patch does only this:

• Fixes the whitespace issue by rendering each link separately and putting a space in between them.
• Fixes the vertical alignment issue mentioned in #259 (screenshots above).

I still don't get the whole design argument on the button (honestly, I don't care at all - I just want D7 out already) but your argument is basically :

1) I don't want to special case it.
2) It might not look good in other themes.
3) Its technically a button.

All of which are technical arguments, not actually design arguments. 1) To me this is a non-argument, because we are already special casing it - would just mean one extra element. 2) We can't help it if people don't style something like this. It not matching with other theme headers is of little concern to me, the same applies to the toolbar which doesn't work well on darker headers. 3) The user does not care what it technically is, if it confuses him/her.

You cannot convince me that from a design perspective this is good, people do not see this as a "button" in the Drupal context (the smashing web article, is not applicable in this context) and will therefor be somewhat thrown off by it showing up like this. Whenever I read something like "sacrificing beauty" for something else, those two words already tell us we are doing it wrong :)

Anyways from a usability perspective, this does not make sense. But I honestly don't really care, if we get to something that looks technically oke - we should just push for commit.

Whatever. Fine I will remove the gray button and give you a blue button for Seven.

It was just a suggestion to avoid adding a completely new design element that does not exist anywhere else in Seven, therefore is not consistent with anything, for a simple one-off message at the 11th hour of the Drupal 7 cycle. It clearly looks and acts like a button to me in #189, but no one else's opinion actually matters...

Standby for a new patch.

• Does this work for screen readers? If we only show the links when the overlay is open, I don't fully understand why we have them in the parent document at all. Whatever that reason is, is this use of keeping the parent one's "display:none", followed by CSS that sets "display:block" only once the overlay
is opened compatible with it?

The point, perhaps a bad call on my part, is to help users be closer to the links when their UA / AT doesn't properly follow the focus being set to the iframe / Overlay child. AFAIK this will work for screen-readers.

• Is the fact that we have JS code setting tabindex=-1 for the parent dom's links if the user is on a modern browsers a problem, with respect to the above? Some screen readers skip over links with tabindex of -1, do they not? Or is it ok, because these same screen readers will find the ones that are in the overlay's dom?

Screen-readers will skip links with tabindex -1, but only when tabbing. Keep in mind that most (AFAIK) screen-reader users don't tab through a web page, they use other reading commands. Again, having the message in the parent is only a hack to assist users when focus is not properly set to the Overlay child.

• Should we also set tabindex=-1 for the parent dom's links in IE6/IE7 too, to match the other browsers? For performance reasons, we can't do this for EVERY link in the parent dom, but we can do it for these two. If we do so, then we don't need to worry about any styling within overlay-parent.css, and it also means that themes not intended to be used as administrative themes wouldn't need to worry bout any styling for these links, since only the ones inside
the overlay would ever be visible. But, would this make these links less accessible to the very users who will likely need them?

Possibly, once again, they are there in case focus isn't properly set to the Overlay child. So, if this happens to a keyboard only user it might be helpful.

This is #273, but modified to make the parent document's message unreachable by tabbing in IE6/IE7 as well. See #265 for more details. I think this significantly improves the experience for theme developers, because it just gives them one message to style, rather than a second one that is only visible in IE6/IE7, and it also means this message is only visible when the theme is an admin theme, which means custom themes for a single site don't have to worry about it at all.

Based on #274, this seems acceptable, since the purpose of the parent document's message is for screen-reader users navigating the document with screen-reader commands that don't follow tab order.

once again, they are there in case focus isn't properly set to the Overlay child. So, if this happens to a keyboard only user it might be helpful.

I don't think this is a problem. Even on IE6/IE7, focus begins in this overlay, so the message in the child will be seen early in the tab cycle. The problem in those browsers is just that after tabbing all of overlay and then the toolbar, instead of cycling back into the overlay, you keep tabbing through all of the parent document. It is at this point, that an IE6/IE7 keyboard-only user will probably:

1) wonder what's up
2) hopefully, remember reading the message that if they're having problems with the overlay, then can disable it
3) hit the browser refresh button (at least that's what i do when i'm on a web page that isn't doing what i think it should be)
4) start hitting tab, and see the message again, and maybe this time, follow the link to disable the overlay

Of course, the above is just a guess. Any real feedback from screen-reader users and keyboard-only users on this patch would be greatly appreciated.

Patch in #276 sounds reasonable to me.

Related, how does a keyboard only user get to the toolbar from where they land in the Overlay child? The real question is is it apparent how they get to the toolbar, does it make sense? Not looking for a long discussion to derail the issue. If there is a problem I'll open a new bug.

The patch seems to work pretty well for me.
I'm using IE 8 if that helps, using JAWS for Windows primarily for testing.
Also as for getting to the admin toolbar it is a heading at level 2 so I just hit h once after logging in and I'm at that heading.
An alturnitive option is to press control-f, type the word toolbar or ever the word "tool" without quotes, and press enter.
That's another way to get there.
Seems extremely clear to me personally how to get to the toolbar.
As for someone that's sighted, they probably drag the mouse and such.
Now if you're just a keyboard-only user without a screen reader program, I have no idea honestly how you might get to the toolbar other than control-f in IE, and finding the text "toolbar" or something.
Any comments on this are appreciated.

Much agree with #281, lets get something in...then we can unblock release candidate and you can always fix during RC or even after actual release. For those who need to wait for this they can use the tried and true tabs that are implemented in virtually ever browser to keep the page they were on while they do administration...lets worry about something else.