Foundation-sites: [Feature Request] Hollow button group

Created on 26 May 2017  路  18Comments  路  Source: foundation/foundation-sites

How to reproduce this bug:

  1. Step one

Create a BUTTON GROUP - The docs provide for All basic types of coloring - Alert, warning etc.
However - an important basic button coloring -- "HOLLOW" is not available for the BUTTON GROUP.

IT IS AVAILABLE for granular button - which is cool, But it would be nice to be able to
assign it to entire button group also if desired without having to insert a class = "button primary hollow" to every button in the group.

What should happen:

class = "button-group primary hollow"

Primary Colored Border - Transparent fill

What happened instead:

No Hollow Available for button group that renders like a HOLLOW SHOULD

Browser(s) and Device(s) tested on:

ALL current

Test case link:

Button Group Hollow Buttons PR open feature request

All 18 comments

I already encountered the situation when I needed this and I agree that could be useful!

The only thing is we probably need some fine tuning about the 1px margin that separates buttons since it looks weird with hollow buttons (or that's probably just me but I'd like to have other people opinion about that).

Here is a CodePen for comparison: https://codepen.io/anon/pen/bWJgWr?editors=1100

(@blynnc I know you are a Foundation fan girl but there is no need for all caps titles ;))

@blynnc @Deckluhm This sounds reasonable (no-gutter proposal also)
Any one of you both want to give it a go and have a PR ??

Just note that we are just nearby 6.4, first RC on coming thursday so make it soon as possible :smile:

Gutter-less wouldn't work very well if you had different colored buttons in your group.

@orangedaisy You have a very good point but we can have .no-gutters class in there and tell In the docs on when to use and when to not?
What do u think??

Update: We use collapse with grid and in 6.4,
we are going to most probably use the term margin-gutters, padding-gutters for grid
And thus no-gutters class will be confusing
So i guess,
no-gaps make more sense to not get confused ??

Sorry but @blynnc @Deckluhm ,,,,, as we are running out of time for 6.4
So, I have decided to take this up myself .... Hope you are ohk with it!

Update: Added a PR #10081

To be honest I was about to take this one but I realized that this wasn't as easy as it seems.

I used "shared borders" in my no-gutter example just for visual comparison but there is one problem with this approach: when you hover a button, the left border color doesn't change as expected (this happens for every buttons except the first one).

The hollow no-gaps example of your PR doesn't have this problem but there is a weird 1px offset on hover and that doesn't feel right.

I think we should explore another strategy: overlapping borders (using z-index). That will require a bit more work but that should avoid both issues.

Why not waiting for the first 6.4.x release to ship this? I mean there is no rush and since it's not a new feature (but more an addition to an existing one), I think it's ok to wait.

@Deckluhm Hmmnnn... Sounds Good .... I will admit i know the issue what you just highlighted
Happy to take a PR and close my PR for the sake of same!
Please have a go if you have a better solution!

Why not waiting for the first 6.4.x release to ship this? I mean there is no rush and since it's not a new feature (but more an addition to an existing one), I think it's ok to wait.

Yes we can wait .... just that i added the PR coz of my unavailability (mostly) from 7th-16th June due to my exams :smile:

Can someone translate PR ? What does it stand for?

@blynnc

  1. That PR adds a button group with hollow as an option too as requested by you
  2. Add a class no-gaps based on @Deckluhm proposal https://codepen.io/anon/pen/bWJgWr?editors=1100

Dang you mean PR full form ... Its a Pull Request
=> https://help.github.com/articles/about-pull-requests/

In short, a "pull request" is a way to submit contribution to a repository.

It offers way to easily review code and (if approved) merge code.

I'd like to add the following: The auto-Setting of the button mixins (e.g. @include button-style($color, auto, auto);) should be variables. Encountered an issue when trying to use css4 color variables for $button-color in settings.scss.

Would be nice if you could have this in mind for the next release ;)

:arrow_double_up: cc @kball

@mixin callout-style($color: $callout-background) {
  $background: scale-color($color, $lightness: $callout-background-fade);

  background-color: $background;
  color: color-pick-contrast($background, ($callout-font-color, $callout-font-color-alt));
}

Same goes for this part...

$callout-font-color uses $body-font-color (like most other mixins?!) which in my case is a css variable and that breaks those mixins easily.

Could you please consider this in the next release ?!

Thanks for all!

@developer-lindner Would you like to do a PR ??

@IamManchanda I'll try to as soon as find some spare time...

Some cleaning and refactor seems required here.

  • At first, there is already some logics to generate buttons _styles_ (solid, hollow and clear) classes. We should use reuse it for button groups.
  • Also, we should not generate these classes when their styles is already the default one.
  • Finally, I think there is some CSS duplication we could remove between buttons and button groups.

And as you already know, I love refactoring ;)

#10081 is merged. It should be released in v6.5.0.

Was this page helpful?
0 / 5 - 0 ratings