Clean up the CSS for Dashboard module

Fixed link to initiative page.

Here's my first shot at this patch. In the end there was only a tiny bit of css needed to make dashboard work as most of it is controlled by jQuery UI.

Also, I changed some of the hardcoded ids to classes. I wanted to convert all of the ids to classes but I couldn't change the #dashboard ID. When I did the dashboard broke.

I think there is an argument to be made here that the dashboard.base.css is too sparse. As the dashboard's usability is removed if you only use dashboard.base.css and any theme would exclude the dashboard.theme.css would have to implement many of the same styles to get to a working state.

If the instructions above are read literally, the dashboard.base.css should be as sparse as possible. That is the aim of this patch.

After having a conversation with Jacine it's clear that I'll need to rework this patch. Admin specific css should be targetted at dashbaord.admin.css. In this case that's a lot of the styles that are in dashboard.theme.css after this patch is applied.

So, this is a good starting point but more work is needed. I'll likely have more time to look at this later in the week.

I just had a quick look at your patch, so here is some feedback that you could incorporate in your updated patch:

The separation between base.css and theme.css seems ok to me, although most of theme.css content should go to admin.css, as you mentioned. In fact, since dashboard is an admin tool I would think that a theme.css is not required here?

Maybe a minor thing we could fix while we are at it is the use of underscores in the dasboard id's. I did not find a convention against that, but I've never seen underscores in other Drupal modules, so we should avoid it here too.

Good luck with the patch!

I'll try to get this done before the next html 5 sprint starts.

Here's a revised patch that admittedly doesn't cover all of the issues we care about in this patch (doesn't cover rtl, but I don't see how we can)

It doesn't seem that the module is specifically calling the rtl.css for this module. Can someone explain what needs to be done in order to have rtl support?

Also, how does the patch look?

@cosmicdreams: just some info without looking at the patch:

Drupal automatically picks up -rtl.css files when needed. That means that if you have a dashboard.theme.css, it will automatically look for dashboard.theme-rtl.css when browsing the site in a RTL-language. That dashboard.theme-rtl.css file will then be added after dashboard.theme.css.

This looks good to me.

+++ b/core/modules/dashboard/dashboard.moduleundefined
@@ -588,7 +588,8 @@ function dashboard_update() {
+  drupal_add_css(drupal_get_path('module', 'dashboard') . '/dashboard.admin.css');
+  drupal_add_css(drupal_get_path('module', 'dashboard') . '/dashboard.theme.css');
   return '<div id="dashboard" class="clearfix">' . $element['#children'] . '</div>';

This doesn't make sense if we are not renaming the original css files.

-22 days to next Drupal core point release.

Working on this again. This is before I properly segregate the styles into .admin.css and .theme.css

This patch refactors dashboard.base.css into two files and eliminates dashboard.theme.css. No CSS has been removed from original implementation yet.

I've tested this with the dashboard.admin.css turned off and it is technically functional but it is quite awkward with the proper paddings and dotted boarders present.

Putting into needs review to test my patch only. Will attempt to prune the css used even further.

In the last patch, I forgot to refactor the rtl styles. Fixed that with this patch.

last patch did not have proper endlines on two files. Fixing...

I've been thinking about making a concerned effort to reduce the amount of css used here (specifically targeting the browser specific properties, and other styles that in testing can be removed without degradation of appearance). But I want to test my assumptions can someone review this and let me know if the patch is good to go now, or if a further css cleanup is necessary?

"module.admin.css
Should hold styles that are only applicable to administrative pages."

Everything from the dashboard module should be part of admin.css, we don't need base.css in this case.

ps: I still think this module is going away soon, so we shouldn't waste much time on this one.
ps2: wrong issue number in the patch name

@aspilicious: it is not because something is only visible in the backend that everything should be in .admin.css. base.admin.css is making sure it works. .admin.css adds styles to that. I'm quite sure this is what Jacine once pointed out.

So I would tend to agree with the patch from #16.

It looks like the only clean up that is going on here is moving selectors into the right file. I'm fine with that because I think this module really needs to be removed from core.

As far as which files to use, since this is strictly for an administrative UI, the styles belong in .admin.css. I know it's confusing for some of these newer modules, but it's different from say, contextual links because those are displayed both on the front and backend. Also things like tabledrag styles, and some of the other stuff in system.base.css can be implemented by modules anywhere, which also makes it "special." That's not going to happen for this module, and we decided early on not to put that kind of energy (splitting admin in the .base and .theme) early on because it's just not worth the effort it would require this time around.

#950956: Remove Dashboard from core

Unassigned issue so I can focus on D8 initiatives.

Updated issue summary.