Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase reusability of list-inline object #223

Closed
twittkopf opened this issue Nov 5, 2016 · 18 comments
Closed

Increase reusability of list-inline object #223

twittkopf opened this issue Nov 5, 2016 · 18 comments

Comments

@twittkopf
Copy link

To make the list-inline object more versatile, the CSS attr() function and a data attribute could be used to define the delimiter inside the HTML markup, like so:

<ul class="o-list-inline o-list-inline--delimited" >
  <li data-inuit-list-inline-delimiter=", " class="o-list-inline__item">Item 1</li>
  <li data-inuit-list-inline-delimiter=", " class="o-list-inline__item">Item 2</li>
  <li data-inuit-list-inline-delimiter=", " class="o-list-inline__item">Item 3</li>
</ul>
.o-list-inline--delimited {
  font-size: 0;

  > .o-list-inline__item {
    font-size: $inuit-global-font-size;
    font-size: 1rem;
  }

  > .o-list-inline__item + .o-list-inline__item {

    &:before {
      content: attr(data-inuit-list-inline-delimiter);
    }

  }

}

This conforms to the CSS 2.1 usage of attr() which is supported in basically every browser.

I think this could be a nice way to change the base object without really changing the base object, therefore increasing reusability of the list-inline object. It would even be possible to use $inuit-list-inline-delimiter as a fallback in case no data attribute is present. In terms of simplicity, searching the delimiter in the markup before taking a look at the CSS would be quite intuitive I believe, especially for people at the beginning of their learning curve.

The only downside to me is that it's not possible to add a space before the delimiter, which unfortunately narrows use cases quite drastically I guess.

Still I can imagine cases where e.g. a pipe character is used as delimiter and the list items contain links, which then get padding applied from a component. If the delimiter is supplied by the list-inline object like proposed above, the pipe delimited list could avoid reimplementing the behaviour from list-inline, only to use a different delimiter. To make my point clear, I created two examples: Example A, list-inline is not used, behaviour is reimplemented and Example B, list-inline can be used.

@hayatbiralem
Copy link
Contributor

This is interesting approach, thanks but I think it is not necessary.

Maybe new pseudo content utilities fits your needs and those will be useful for any different possibilities. You can use utilities with other objects and components.

A pen for my opinion here and related codes are below.

CSS

/* ==========================================================================
   #FOOTER-NAV
   ========================================================================== */

.c-footer-nav__link {
  padding: 12px;
  color: red;
}


/* ==========================================================================
   #CONTENT-(BEFORE|AFTER)
   ========================================================================== */

/**
 * Pseudo content utilities.
 */

[data-u-content-before]:before {
  content: attr(data-u-content-before) !important;
}

[data-u-content-after]:after {
  content: attr(data-u-content-after) !important;
}

HTML

<ul class="o-list-inline c-footer-nav" >
  <li class="o-list-inline__item c-footer-nav__item">
    <a class="c-footer-nav__link" href="#">Link 1</a>
  </li>
  <li class="o-list-inline__item c-footer-nav__item" data-u-content-before="|">
    <a class="c-footer-nav__link" href="#">Link 2</a>
  </li>
  <li class="o-list-inline__item c-footer-nav__item" data-u-content-before="|">
    <a class="c-footer-nav__link" href="#">Link 3</a>
  </li>
</ul>

What do you think about this?
It's more optimal, isn't it?

@twittkopf
Copy link
Author

I like the idea of using utility classes for this. It suits the intention really well – since the delimiter is actually not part of the content, it seems reasonable to regard it as implementation detail.

On the other hand, theoretically speaking, including the delimiter as part of the framework might be a bit opinionated then, don't you think? If it's okay to have a comma in there, why not re add the bordered list of previous inuitcss versions? Practically it's probably not such a problem since a) it's just a comma after all and b) it's bound to a modifier class one can just leave off. However it seems kind of unclear where to draw the line. Maybe the delimited list would be more suitable as a component?

@hayatbiralem
Copy link
Contributor

I think you are right. IMO the delimited inline list is unnecessary, I never use it before but if I need to delimited list in the future probably I'll going with custom component.

Maybe we must remove delimited modifier from list-inline object. What does @inuitcss/core think about this issue?

@herzinger
Copy link
Contributor

I don't think anything should be changed. The current approach is good enough for most implementations, so it's okay for the framework, and both your solutions are fine in a project level if you need it.

@twittkopf
Copy link
Author

Not all languages use a comma to separate list items. Even if $inuit-list-inline-delimiter can be used to set a different delimiter at build time, this could add quite some complexity to projects where a separate build for each language is created. I think it would be better to deal with that complexity at component level and not at object level.

In my opinion, things like this should be considered since inuitcss is a framework for large projects.

@herzinger
Copy link
Contributor

herzinger commented Nov 9, 2016

@twittkopf the comma is just a placeholder. If you gonna use the object, you'll want to predefine the variable in the settings part of your project.

i.e.: I use it with >, for breadcrumbs

There is one think I like in the attr() approach: that way you can have different types of delimiters in the same project from the get go, but them you HAVE to set the data attribute every time you use it. You could achieve that with an utility class or another modifier just as easy and in a more seamless way. Heck, you could even have something like...

.u-content-before:before { content: attr(data-inuit-content-before) !important; }
.u-content-after:after { content: attr(data-inuit-content-after)  !important; }

...and use it only where you want an html customizable separator (and it can be used in a lot more ways, unrelated to this issue). But if all you want is one or two alternative separators, one specific class for each alternate version is a lot better.

@herzinger
Copy link
Contributor

Does anyone from the @inuitcss/core have anything to add?

@csshugs
Copy link
Member

csshugs commented Nov 11, 2016

I'm not a fan of the attr() approach. There are certainly situations where this technique serves itself useful. But in this case, I think it should be handled with CSS.

I'm also in favor of removing the --delimited modifier. I already wanted to bring this up months ago, but managed to somehow forget about it. Thanks for bringing this up again. This is actually a bit opinionated for inuitcss IMO. I also doubt that it this modifier is actually still an object. It rather feels like a component. Even if not, it's not something that inuitcss should necessarily provide in it's core.

@inuitcss/core What do you think? Do we want to get rid of the --delimited modifier?

@herzinger
Copy link
Contributor

@csshugs yeah. It could easily and arguably more correctly be added in components line c-breadcrumbs or c-tags-clould and so on. I'm in favor of dropping it.

@csshugs csshugs modified the milestone: 6.0.0-beta.5 Nov 11, 2016
@nenadjelovac
Copy link
Member

@csshugs

@inuitcss/core What do you think? Do we want to get rid of the --delimited modifier?

agreed.

I'm not a fan of the attr() approach. There are certainly situations where this technique serves itself useful.

and agreed again ;)

@florianbouvot
Copy link
Member

@inuitcss/core What do you think? Do we want to get rid of the --delimited modifier?

I use it in a lot of project but most of the time it's not ideal because I need more than one delimiter "design".

@herzinger
Copy link
Contributor

I use it in a lot of project but most of the time it's not ideal because I need more than one delimiter "design".

That is the exact situation that created this issue. The way it is today, it's not very reusable for an object.
All 3 approaches presented here address this nicely (attr(), content utilities and specific components.

@anselmh
Copy link
Member

anselmh commented Nov 13, 2016

Can we bring this up as separate plugin (in its own repo)? I’d love to not break the code for many users without offering them a chance to migrate.

@herzinger
Copy link
Contributor

@anselmh sure, I can even do it myself. But with which approach?

@csshugs
Copy link
Member

csshugs commented Nov 15, 2016

@herzinger

I can even do it myself

Thanks for the offer. But please let @inuitcss/core first agree internally on a convention for providing plugins and then offering this specific module as an official inuitcss plugin.

@herzinger
Copy link
Contributor

@csshugs yeah, sure. I said it before, but I have at least 3 other plugin ideias waiting for that resolution now.

@csshugs
Copy link
Member

csshugs commented Nov 15, 2016

@herzinger
Cool, nice to hear ✊

Please be patient 😄

@csshugs
Copy link
Member

csshugs commented Dec 2, 2016

Since we removed the --delimited modifier from .o-list-inline in #231, I'm closing this.

@csshugs csshugs closed this as completed Dec 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants