Skip to content
This repository has been archived by the owner on Dec 11, 2021. It is now read-only.

Variables: Initial pass on button variables #138

Closed
wants to merge 19 commits into from

Conversation

sfrisk
Copy link
Contributor

@sfrisk sfrisk commented Nov 24, 2015

Based on discussions from last week's meeting, here is an initial pass at buttons' variables to help facilitate the discussion on the best practices for organizing and naming variables for jsass.

"padding": ".35em .75em"
}
},
"ui-btn-xs": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line seems to have incorrect indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@sfrisk
Copy link
Contributor Author

sfrisk commented Nov 24, 2015

One of the questions brought up in today's meeting is do we want to use maps as much as I have in this example, or is it better to split things up more than what I did and have more variables and/or more specific maps.

@cvrebert
Copy link
Contributor

Perhaps there should be btn-md and btn-default modifier classes, instead of setting a default size and color on btn which then get cascaded and overridden by the existing size and color modifier classes?

@arschmitz
Copy link
Contributor

@cvrebert +1 we have a goal of avoiding cascading and specificity

@sfrisk
Copy link
Contributor Author

sfrisk commented Dec 15, 2015

yeah, good point. Part of me didn't want to have additional classes, but overwriting is also icky.

@geekman-rohit
Copy link
Contributor

http://view.css-chassis.com/96-buttons-take-two/demos/buttons.html
@sfrisk a little shadow? The smaller ones look more like labels than buttons.

@sfrisk
Copy link
Contributor Author

sfrisk commented Jan 19, 2016

I'm not entirely thrilled with the design, playing around with it more, just figured I would post what I had so far.

@geekman-rohit
Copy link
Contributor

I kind of like the mixins and variables you made. I guess colors/design can wait until we have a few more basic things merged

value: {
"font-weight": 500,
"margin": ".25em",
"text-transform": "uppercase"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geekman-rohit --- this is what I was talking about in #139

@sfrisk
Copy link
Contributor Author

sfrisk commented Jan 26, 2016

This branch is now using the palette shown in #140

@sfrisk
Copy link
Contributor Author

sfrisk commented Feb 9, 2016

@geekman-rohit
Copy link
Contributor

Ship It. Looks Really Good.!

@sfrisk
Copy link
Contributor Author

sfrisk commented Feb 16, 2016

Button PR has been updated: http://view.css-chassis.com/96-buttons-take-two/demos/buttons.html

Added block buttons, button bar, some hover/active/etc colors for the default button, anchor button demo, and input[type=submit] button demo.

@include btn-size($ui-btn-md);
}

.btn-group {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably do this differently so it doesn't rely on cascading markup

Maybe .btn-group--btn on the interior buttons? My only concern there had been there could easily be too many .btn classes on the button elements

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we probably shouldn't rely on cascading markup. Normally in a scenario like this I wouldn't think twice about writing it that way but sense we are writing this with a focus on being easy to override we may want to stick to the one class approach.

To avoid the too many classes problem maybe we could write a mixin for the main button styles and then include it on the .btn and the .btn-group--btn classes? Not sure if that's overkill but it's just a thought.

@sfrisk
Copy link
Contributor Author

sfrisk commented Feb 19, 2016

Pinging @geekman-rohit @arschmitz @kristyjy for reviewal. Does this approach look good for buttons and what we wanted for naming conventions?

@kristyjy
Copy link
Contributor

This looks great to me other than maybe giving the cascading button groups a little more thought. If we do decide to keep the cascading we may want to add notes to the style guide to make sure we have guidelines for deciding when we should and shouldn't cascade.

@sfrisk
Copy link
Contributor Author

sfrisk commented Mar 1, 2016

Update made, removed the need for cascading.

}

@mixin btn-disabled($btn-colors) {
cursor: default;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cursor property for a disabled button can be a not-allowed, will this be better ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfrisk what @thejdeep said. I think it should be not-allowed too. There should be a variable too for that.

},
"chassis-gray": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bring back gray colors to palette!!!

@sfrisk sfrisk closed this in 4d3769f Mar 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants