Remove/fix some design choices

[silvenon]

I believe there are some design choices baked in inuit.css that should be looked into.

• [x] base/_links.scss

There are many cases I wouldn't want to underline links at all, so I have to cancel those rules. Also, below there's a rule which sets cursor to text on hover if it's a .current link, which doesn't work well if there are images instead of text (text cursor + images = bad UX). Nevertheless, even with text links, it's not something everyone would like, some people prefer default cursor. #93

I suggest removing that component altogether.

• [ ] generic/_reset.scss

Maybe changing the cursor on hovering form elements is best to leave to the designer. Using pointer on hover for button elements is perfectly fine, but I don't think using it for text input elements is a common pattern.

• [x] base/_images.scss

Setting height to auto prevents images to ever care for the HTML height attribute.

• [ ] base/_paragraphs.scss

I think the multiplier should be configurable.

• [x] objects/_icon-text.scss

I think the margin should be configurable.

• [ ] Other

I think symbols like "»" (for breadcrumbs and other) should be configurable.

Well, that's all for now, let me know what you think ;)

[andykirk]

Agreed. The links issue is similar to the one I raised.

It's my belief these things should be set as default variables so they can easily be changed without having to override them or comment out the scss file containing them.

Cheers.

[silvenon]

Agreed. The links issue is similar to the one I raised.

Yep, that's why I (not very obviously) referenced your issue :)

[andykirk]

Sorry, yeah - didn't spot that...

[silvenon]

I found a pretty dangerous one:

img{
    max-width:100%;
    height:auto;
}

Setting height to auto prevents images to ever care for the HTML height attribute.

[csswizardry]

@silvenon This declaration is needed to make fluid images maintain their aspect ratio; without it, images would distort

Try resizing the Result pane in the linked fiddle, then—using Inspector or similar—remove height:auto; and resize again :)

[silvenon]

You're right, though I think there should be a less destructive way of achieving this. Perhaps applying those only on .fluid images or something.

HTML dimension attributes are really important to me because they prevent jumps while the page is loading.

[silvenon]

This seems like a longshot, but maybe it could work:

img:not([width]):not([height]) {
  max-width: 100%;
  height: auto;
}

This is one StackOverflow answer to my question.

[csswizardry]

If you, in arguably the minority, don’t want fluid images as standard then just set max-width:none; in your theme stylesheet: http://jsfiddle.net/Fdkqc/1/

[silvenon]

I want to avoid the visual jump when the images are loaded. When <img> elements have size attributes set, they take up space before they are loaded, which is simply a better experience. This way they will only take up width, not height.

Fluid images are simply impossible to cancel, that's why I think the selector should be more specific.

[teddyzetterlund]

I don't have a strong opinion regarding if inuit should make the change (I don't use inuit), but I'll leave this here anyways since I think the issue has a good point:

Specifying a width and height for all images allows for faster rendering by eliminating the need for unnecessary reflows and repaints. — Optimize browser rendering – Make the Web Faster – Google Developers

Especially relevant since there's quite a few discussions about page rendering having a big usability impact. Personally, however, I realize it depends quite a bit between cases and it's up to Harry what preference inuit should opt for.

[silvenon]

That's a good point. Imagine hundreds of avatars one next to the other, they don't need to be responsive and setting size for them would help performance.

If I didn't like the images module at all, I wouldn't include it, no problem, but the thing is that I want it selectively.

[csswizardry]

I’ve committed a change which should make responsive/fluid images optional: https://github.com/csswizardry/inuit.css/commit/89cbff82652bf440f0782e8a8719a7da284a2bcf

Let me know if that’s okay :)

[andykirk]

Hi,

Just to add to this discussion, in agreement with the OP I feel that many of the places where you specify "override this in your theme stylesheet..." (e.g. .pill) should instead be configurable via variables, as overriding can sometimes be a bit of a pain. One of the main appeals of this framework is that you (mostly) don't have to override stuff. The places where you do then become more nigglesome.

(Incidentally, I'm working on a bootstrap-esque) design layer for my own use that sits alongside Inuit and it's entirely configurable via variables. It's working out ok so I plan to make it available if anyone is interested? Note it is a separate thing, and not meant to interfere with Inuits design-less approach.)

Anyway, I digress. In addition to the few places where I think there should be extra variables, I've come across a situation with .nav (in relation to .breadcrumb) where I think the framework falls short in terms of flexibility and goes against some of the OOCSS principals. Breadcrumbs assumes you're using a ol/ul to mark up the breadcrumbs, but I actually prefer to use a dl. The best markup for breadcrumbs is debatable and comes down to personal choice, but isn't this framework meant to allow you to use it without having to adhere to specific markup? Wouldn't it be a good idea to provide classes instead of (or as well as?) relying on specific elements?

So for example I'd like to have something like this:

<dl class="nav  breadcrumb">
    <dt class="nav__item">Your are here:</dt>
    <dd class="nav__item  breadcrumb__root"><a href=#>Home</a></dd>
    <dd class="nav__item"><a href=#>Somewhere</a></dd>
    <dd class="nav__item"><a>Current</a></dd>
</dl>

and update nav to something like this:

.nav{
    ...
   & > li, & > .nav__item{
        ...
    }
}

.breadcrumb would need to be tweaked too but that might mean needing a .breadcrumb__item class too?

Just a few thoughts. What do you think? Happy to put in the work on this via a fork or something (I'm new to git so still finding my way)

Cheers (and sorry for such a long post)

[silvenon]

:+1:

Pagination module has a similar problem. I'm using will_paginate, so the markup simply doesn't match. But that's a lot hard to work around :P

An unrelated question: why are there sometimes underscores and sometimes hyphens? I haven't switched to SMACSS yet, but it says here that underscore represents a submodule. I don't see any double underscores.

[tlgreg]

@silvenon It's the BEM naming convention, separators are doubled, underscore means child block, hyphen means modifier.

And I agree, inuit could increase the number of configurable variables on these topics.

[teddyzetterlund]

@silvenon You might be interested in this thread about overriding will paginate's LinkRender.

[andykirk]

Oh, on the topic of pagination, I had expected to see .pagination__prev and .pagination__next classes, or am I missing something? Also, perhaps a .pagination__counter class to denote "Page X of Y" sort of text, maybe?

[silvenon]

@teddyzetterlund Thanks :)

[caleb]

I agree that changing the cursor for form elements is confusing. I think the browser defaults are best since it's what most people expect.

type text for text boxes and the default arrow pointer for buttons, select boxes, etc.

[silvenon]

cursor: pointer is great for buttons, because browser defaults suck there. I know I hate when I hover over a button and cursor remains default.

[caleb]

That's fine with me. So long as text boxes don't use pointer I'm happy.

[silvenon]

Hahaha :D

[csswizardry]

The bulk of this has been addressed in varying degrees. I’m gonna close it, but feel free to open other, more specific issues if needs be.