Clean up the CSS for Overlay module

i was researching this issue and noticed that in overlay-child-rtl.css, this is in the css:

#overlay-tabs li {
  margin: 0 -3px 0 O;
}

I think that is the letter O?

Is that some weird css hack or is it a mistake?

Ha! Nice catch. Yeah, definitely looks like a bug.

Probably better to file it as a separate issue, though, because that's a simple fix (whereas this issue is about more far-ranging changes to the CSS).

ok cool, i filed it under #1326950: typo in overlay-child-rtl.css

I'll take a shot at this. Patch forthcoming

Got started, There are a lot of IDs here:

<?php print render($disable_overlay); ?>
<div id="overlay" <?php print $attributes; ?>>
  <div id="overlay-titlebar" class="clearfix">
    <div id="overlay-title-wrapper" class="clearfix">
      <h1 id="overlay-title"<?php print $title_attributes; ?>><?php print $title; ?></h1>
    </div>
    <div id="overlay-close-wrapper">
      <a id="overlay-close" href="#" class="overlay-close"><span class="element-invisible"><?php print t('Close overlay'); ?></span></a>
    </div>
    <?php if ($tabs): ?><h2 class="element-invisible"><?php print t('Primary tabs'); ?></h2><ul id="overlay-tabs"><?php print render($tabs); ?></ul><?php endif; ?>
  </div>
  <div id="overlay-content"<?php print $content_attributes; ?>>
    <?php print $page; ?>
  </div>
</div>

I started going through the process of reducing the number of ids. Here's what I'm considering after the refactor:

<?php print render($disable_overlay); ?>
<div id="overlay" <?php print $attributes; ?>>
  <div class="titlebar clearfix">
    <div class="title-wrapper clearfix">
      <h1 class="title"<?php print $title_attributes; ?>><?php print $title; ?></h1>
    </div>
    <div class="close">
      <a href="#" class="overlay-close"><span class="element-invisible"><?php print t('Close overlay'); ?></span></a>
    </div>
    <?php if ($tabs): ?><h2 class="element-invisible"><?php print t('Primary tabs'); ?></h2><ul class="overlay-tabs"><?php print render($tabs); ?></ul><?php endif; ?>
  </div>
  <div class="content"<?php print $content_attributes; ?>>
    <?php print $page; ?>
  </div>
</div>

In my view, there should be only one id, "overlay" for the element that begins and encapsulates the other things that makeup the overlay.

Then I need to adjust the styles accordingly. Sound like a good plan?

Also, should we be concerned about css namespacing issues? Like I totally see another module make an overlay and what to use the css id "overlay" for it. Should we name css ids in core like this: drupal-overlay?

My focus has been elsewhere in regard to HTML5 patches. But I'm bumping this so I can work on this issue tonight. I hope to produce a patch based on the plan I out lined above.

My focus has been elsewhere in regard to HTML5 patches. But I'm bumping this so I can work on this issue tonight. I hope to produce a patch based on the plan I out lined above.

OK, so.... here's a patch that may probably will get conversation started. Here's a rundown of what I did:

1. sorted overlay-child.css into two css files (base and admin)
2. modified the markup for overlay to only have one div
3. modified the javascript to respect the one div
4. modified all other parts of the module to also respect the one div
5. Help: couldn't find the tests for overlay

+++ b/core/modules/overlay/overlay.tpl.phpundefined
@@ -22,16 +22,16 @@
-  <div id="overlay-titlebar" class="clearfix">
-    <div id="overlay-title-wrapper" class="clearfix">
-      <h1 id="overlay-title"<?php print $title_attributes; ?>><?php print $title; ?></h1>
+  <div class="titlebar clearfix">
+    <div class="title-wrapper clearfix">
+      <h1 class="title"<?php print $title_attributes; ?>><?php print $title; ?></h1>

You're not rly removing any div's. So the markup is not rly changed.
And we need a way to test "function overlay_disable_message()".
I'll functional test this some more today or tomorow.

23 days to next Drupal core point release.

Whoops, needs review

#9: 1217030-9-overlay.patch queued for re-testing.

@aspilicious ,
if you can, please write a test for function overlay_disable_message() in fat module

Didn't change the css filenames to the new convention - just went through and swapped out the ids for classes.

This patch was created as a part of the code sprint at #dcATL.

#15: overlay-change-ids-to-classes-1217030-15.patch queued for re-testing.

Updated summary to the latest CSS organization guidelines.

Overlay is dead to D8 #2088121: Remove Overlay.

We cannot change this in 7.x now.