Clean up tour.module.css

1. +++ b/core/modules/tour/css/tour.module.css
   @@ -16,7 +16,7 @@
   -.toolbar .tour-toolbar-tab button.is-active {
   +.toolbar .tour-toolbar-tab .is-active {
      background-image: -webkit-linear-gradient(rgba(255, 255, 255, 0.25) 20%, transparent 200%);
      background-image: linear-gradient(rgba(255, 255, 255, 0.25) 20%, transparent 200%);
    }
   

   I have no idea why this code exists and it's overwritten by the toolbar module CSS anyway, we shouldn't be changing the look and feel of toolbar components in another module.

   We can delete this CSS.

2. +++ b/core/modules/tour/css/tour.module.css
   @@ -47,7 +47,7 @@
   -#joyRideTipContent {
   +.joyride-tip-content {
   

   I can't find this ID or this class in the markup? I wonder if we can delete this without any effect.

I'm also uploading the css file so it can reviewed using Dreditor

1. .toolbar .tour-toolbar-tab button {
     padding-bottom: 1em;
     padding-top: 1em;
     color: #fff;
     font-weight: bold;
   }
   ...
   .tour-toolbar-tab button:focus {
     outline: thin dotted;
   }
   

   None of these properties have any affect, and we shouldn't be doing this anyway, we should remove it.

2. /* @todo Remove once http://drupal.org/node/1916690 is resolved. */
   .toolbar .tour-toolbar-tab.toolbar-tab.hidden {
     display: none;
   }
   

   I'm not sure when the 'hidden' class is added but this is just a clone of CSS that already exists in the toolbar module CSS. We should delete this.

3. /* @todo Remove once http://drupal.org/node/1916690 is resolved. */
   .toolbar .tour-toolbar-tab.toolbar-tab.hidden {
     display: none;
   }
   

   This is an accidental copy of the CSS above and can be removed as well

Patched as per the suggestions form @LewisNyman above.

I noticed the first change of #6 is a typo:

-/**
+fv/**

Also removed :

.toolbar .tour-toolbar-tab .is-active {
   background-image: -webkit-linear-gradient(rgba(255, 255, 255, 0.25) 20%, transparent 200%);
   background-image: linear-gradient(rgba(255, 255, 255, 0.25) 20%, transparent 200%);
 }

as Lewis suggested in #3.

When running csslint, I still noticed a warning:
[L34:C3] Using width with padding can sometimes make elements larger than you expect. Don't use width or height when using padding or border. (box-model)
And this was about this:

.joyride-tip-guide {
  position: absolute;
  display: none;
  background: #fff;
  width: 300px;
  z-index: 101;
  top: 0; /* keeps the page from scrolling when calculating position. */
  left: 0;
  padding: 1em 1em 1.5em 1.5em; /* LTR */
}
[dir="rtl"] .joyride-tip-guide {
  padding: 1em 1.5em 1.5em 1em;
}

I also removed some unnecessary comments and moved a media-query to its appropriate place.

@lewis: Is this a ticket where we change some classnames to be more readable?

@lewis: Is this a ticket where we change some classnames to be more readable?

No, but we are a little stuck on the classes that the joyride library supplies.

We just need some screenshots from Stark and Seven to show that it still looks good.

I'm at DrupalCon LA I'll make the screenshots

Adding screenshots, side by side on the left the patch on the right 8.0.x

Nice work, thanks for the screenshots!

Thanks for the work on this!

I mentioned that there was still 1 cssLint error on the boxmodel.
Thats is why I tweaked the approved patch a tiny little bit, so also the last error is solved. => see interdiff.

The only visual change I see is when there is not a lot of text inside a popup, everything will be more compact (but without breaking.)

Please make before and after screenshots to show what is mentioned in #16

Seems like there is still work. The closing arrow doesn't look good when at tablet breakpoint. The more compact layout doesn't look good, since the x is to close to the text.

Here's the modal dialog from the Seven styleguide.

Added screenshot-with-patch.png that shows before and after side by side.
Addedwith-pach.png that shows after.

Thanks for the screenshots sqndr!

So: The underlying problem is that sometimes with width: 300px; instead of max-width: 300px; this will also sometimes happen. But not so often. That is why my suggestion would be to add more padding so the spacing stays correct. next to the close-button.

Also the adding of a min-width would be good so with very short titles & text, the button doesn't overlap with the pager-thing in the bottom-right

Placed the width of 300pixels back but styled the rest in a way that it no longer depends on its container width to not break.

Needs another round of review!

Sorry I didn't get a chance to test it until now, it needs a reroll.

There were some comments that were now using a different notation of a drupal.org link and therefor the patch no longer applied.

Setting back to needs review.

Great thanks. The code looks good, here are some screenshots from Stark and Seven.

I did some manual testing of this patch nothing appeared to be broken any more than it already is. We need to check if issues exist for the following issues and if not create them:

1. +++ b/core/modules/tour/css/tour.module.css
   @@ -58,30 +29,22 @@
   +  .joyride-tip-guide {
   +    width: 85%;
   +    left: 2.5%;
   +  }
   

   This is currently completely broken when the toolbar is down the left hand side of the screen. However it is broken in exactly the same way in HEAD.

2. RTL tours point to the left side of the element when they should point to the right side. This is easiest to see in the views edit form tour step 2.

Committed aa19ecf and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the critical.