wet-boew / wet-boew-styleguide

A style guide for the Web Experience Toolkit.
http://wet-boew.github.io/wet-boew-styleguide/index-en.html
36 stars 32 forks source link

Can't distinguish links that word wrap from those on new lines #87

Closed cfarquharson closed 10 years ago

cfarquharson commented 10 years ago

The style guide has been updated with the new 'Related' effect, per #76 but I noticed that links that word wrap don't have an different in vertical spacing from those on new lines.

Do we need a class that margin-bottoms lists by default? This is what is currently happening link2

@rubinahaddad @pjackson28 @shawnthompson

pjackson28 commented 10 years ago

@cfarquharson Is a separate class needed or can we just fix the padding for the existing CSS for this area?

cfarquharson commented 10 years ago

Not sure how we'd target it @pjackson28 ? Check out this page: http://wet-boew.github.io/wet-boew-styleguide/v4/design/forms-en.html It just uses existing CSS

pjackson28 commented 10 years ago

@cfarquharson Why is list-unstyled being used here? The purpose of the bullets is to make it easy to distinguish between each list item. It looks weird without bullets as it doesn't look like more than a blob of text (and indenting is weird).

shawnthompson commented 10 years ago

Won't you have the same problem on all list of links that wrap lines? We've always had this problem with links that have a lot of text and that wrap. On ServiceCanada.gc.ca we created a utility style to space out list that would wrap instead of applying a class to every li we created a class to add to the ul to give the list items inside more margin on the bottom.

cfarquharson commented 10 years ago

@shawnthompson what was the CSS for that?

shawnthompson commented 10 years ago

It wasn't pretty and could use a little fine tuning, like adding child selectors....

<ul class="spacedList">
<li>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
quis nostrud exercitation ullamco laboris nisi ut aliquip</li>
<li>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
quis nostrud exercitation</li>
<li>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
quis nostrud exercitation ullamco</li>
</ul>
ol.spacedList li, 
ul.spacedList li{
    margin-top: 15px;
    margin-bottom: 15px;
}
ol.spacedList li li, 
ul.spacedList li li {
    margin-top: 0;
    margin-bottom: 0;
}
cfarquharson commented 10 years ago

@pjackson28 @shawnthompson How about this?

.lst-spcd > li {margin-bottom:10px;}
.lst-spcd  ul,.lst-spcd  ol {margin-top:10px;}

It just tarets the first level. If you want it on other levels, then add it to each nested <ul>

For example:

<div class="well">
    <h2 class="h3 mrgn-tp-0">Related </h2>
    <ul class="list-unstyled lst-spcd">
     <li><span class="fa fa-picture-o text-muted"></span> <strong>Design guide</strong>
      <ul class="list-unstyled mrgn-lft-md lst-spcd">
       <li><a href="buttons-en.html">Buttons<span class="wb-inv"> - Design guide</span></a></li>
       <li><a href="http://wet-boew.github.io/v4.0-ci/demos/formvalid/formvalid-en.html">Form validation<span class="wb-inv"> - Design guide</span></a></li>
       <li><a href="http://wet-boew.github.io/v4.0-ci/demos/feedback/feedback-en.html">Feedback form<span class="wb-inv"> - Design guide</span></a></li>
      </ul>
     </li>
     <li><span class="fa fa-wheelchair text-muted"></span> <strong>Accessibility guide</strong>
      <ul class="list-unstyled mrgn-lft-md lst-spcd">
       <li><a href="../accessibility/forms-en.html">Forms<span class="wb-inv"> - Accessibility guide</span></a></li>
       <li><a href="../accessibility/keyboard-en.html">Keyboard and user controls<span class="wb-inv"> - Accessibility guide</span></a></li>
      </ul>
     </li>
    </ul>
   </div>
pjackson28 commented 10 years ago

@cfarquharson What about the bullets, why were they removed when they are a normal convention for list items? It would be one thing to remove the bullets when each list item is in a box or some other delimiter/visible container but why are they being removed in this case? That would help to avoid the bleeding lines and would require less spacing.

cfarquharson commented 10 years ago

@pjackson28 because they are bland and this is a feature box that showcases the related links... Just like how we don't have bullets in the related aside box that will go at the bottom. https://github.com/wet-boew/wet-boew-styleguide/issues/71

pjackson28 commented 10 years ago

@cfarquharson It doesn't look good without the bullets either. It looks weird that the text in the sub-items are further to the left than the text in the parent item.

cfarquharson commented 10 years ago

@pjackson28 That's cuz there are icons there, I could just put mrgn-lft-lg instead of mrgn-lft-md to fix that. Still doesn't solve the original issue raised though.

pjackson28 commented 10 years ago

@cfarquharson The issue you are facing is that Bootstrap removes the bottom margin of a nested list. Another option of resolving this is to take the top items out of the list and turn them into headings. That way you don't need to worry about the spacing as the heading takes care of that for you. They look like headings anyway with the bolding so I'm not sure why they aren't headings markup-wise.

shawnthompson commented 10 years ago

I agree with @pjackson28 that they should be headings but I still think we need something to space out list items by adding a class to the list instead of each of the list items themselves, which I believe was the initial reason for this issue. I like what @cfarquharson suggested.

.lst-spcd > li {margin-bottom:10px;}
.lst-spcd  ul,.lst-spcd  ol {margin-top:10px;}
cfarquharson commented 10 years ago

@pjackson28 just following up per @shawnthompson 's comment above

pjackson28 commented 10 years ago

Why would there need to be a margin-top on the nested list on top of the margin-bottom for each of the list items? Wouldn't that result in 20 pixels of margin between the parent list item and the first nested list item?

cfarquharson commented 10 years ago

Hey Paul, the margin-top is when you have a ul.lst-spcd with a nested ul. Since the nested ul sits inside an <li> then it would but right up to the top of the item since the margin is on the bottom after the closing of the list item.

You are correct that techinically the final <li> in the nested list would have a 10px bottom and so would the parent <li> that it's in, but the browser is collapsing the duplicate margin.

This is just basic demo list with links with the CSS mentioned:

.lst-spcd > li {margin-bottom:10px;}
.lst-spcd  ul,.lst-spcd  ol {margin-top:10px;}

spacing-1

But if you remove this .lst-spcd ul,.lst-spcd ol {margin-top:10px;} then you lose that initial first spacer on the nested list: spacing-2

This is the HTML:

<div class="well">
    <h2 class="h3 mrgn-tp-0">Related </h2>
    <ul class="list-unstyled lst-spcd">
     <li><a href="buttons-en.html">Parent link</a>
      <ul class="list-unstyled mrgn-lft-md lst-spcd">
       <li><a href="buttons-en.html">Sub-link</a></li>
       <li><a href="buttons-en.html">Sub-link</a></li>
       <li><a href="buttons-en.html">Sub-link</a></li>
      </ul>
     </li>
     <li><a href="buttons-en.html">Parent link</a>
      <ul class="list-unstyled mrgn-lft-md lst-spcd">
       <li><a href="buttons-en.html">Sub-link</a></li>
       <li><a href="buttons-en.html">Sub-link</a></li>
       <li><a href="buttons-en.html">Sub-link</a></li>
      </ul>
     </li>
    </ul>
   </div>
pjackson28 commented 10 years ago

Okay, I added your CSS as is.