Fix border-radius not being set on 2 corners of vertical btn group #16684

Merged
merged 1 commit into from Nov 9, 2015

Projects

None yet

4 participants

@kkirsche
Contributor

While this is correct that it was not being set, I did not notice any visible difference in Chrome on OS X when it is not set.

Fixes #16683

@kkirsche kkirsche Fix border-radius not being set on left of vertical btn group
While this is correct that it was not being set, I did not notice any visible difference in Chrome on OS X when it is not set.

Fixes #16683
5387ed6
@kkirsche kkirsche changed the title from Fix border-radius not being set on left of vertical btn group to Fix border-radius not being set on 2 corners of vertical btn group Jun 22, 2015
@cvrebert cvrebert added the css label Jun 22, 2015
@sdwarwick

it doesn't impact the standard view until you make modifications to the less variables that manage corners. For example, I change the following in "variables.less" file in bootstrap 3.3.4 ( note - this is prior to the addition of btn-border-radius-base, but the problem exists for that case as well ) :

-@border-radius-base: 4px;
-@border-radius-large: 6px;
-@border-radius-small: 3px;

  • @border-radius-base: 4px;
  • @border-radius-large: 6px;
  • @border-radius-small: 10px; // my change

this creates a "gumby" kind of look where the border curves are asymmetric..

image

Owner

Ah, I didn't experience it when I changed them via the Customizer with modified variables and built a local demo that used the vertical button group. Fundamentally I agree with you @sdwarwick, I just didn't experience that behavior (which I was expecting to). I made the PR because I don't know why mine didn't do that and as such I don't trust what mine built even though it checked out in regards to values of the resulting CSS

@kkirsche I've commented in the main part of this thread that it looks like it is better just to have no reference to a specific radius, other than setting the zeros. I am not a regular patch contributor, and was wondering if you might update your submission to reflect that concept.

@mdo
Member
mdo commented Aug 5, 2015

Where do we stand with this PR? I don't recall seeing any example demos pointing out the problem, and it seems we need to revisit how we remove the rounded corners as opposed to resetting them?

@kkirsche
Contributor
kkirsche commented Aug 5, 2015

That would seem to be the case @mdo. I haven't been able to replicate the problem though I understand it theoretically should exists the image above would show. It seems to be inheriting the two corners.

@mdo mdo added this to the v3.3.6 milestone Nov 9, 2015
@mdo mdo merged commit ef8bc28 into twbs:master Nov 9, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mdo mdo referenced this pull request Nov 9, 2015
Closed

v3.3.6 ship list #16644

@kkirsche kkirsche deleted the kkirsche:patch-2 branch Nov 9, 2015
@kkirsche
Contributor
kkirsche commented Nov 9, 2015

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment