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

Missing border radius variables for small and large inputs #14511

Closed
hoho opened this issue Sep 2, 2014 · 9 comments
Closed

Missing border radius variables for small and large inputs #14511

hoho opened this issue Sep 2, 2014 · 9 comments
Labels
Milestone

Comments

@hoho
Copy link
Contributor

hoho commented Sep 2, 2014

There is @input-border-radius variable and it is used by default for .form-control.
But for .input-sm and .input-lg border radiuses @border-radius-small and @border-radius-large are used.
This means that I can reset border radius for default inputs, but for small and large inputs I have to reset radiuses globally (and that's not what I want).

I can create pull request and add @input-border-radius-small and @input-border-radius-large variables having @border-radius-small and @border-radius-large values by default.

Is it ok?

@cvrebert cvrebert added the css label Sep 2, 2014
@mdo
Copy link
Member

mdo commented Sep 4, 2014

Definitely open that PR. It's a simple change we can add I think. Unsure about timing though and if it fits into the v3.2.1 release or if we'd have to do a v3.3 for it. Maybe @cvrebert has ideas?

@hoho
Copy link
Contributor Author

hoho commented Sep 5, 2014

One question.
Current variable name is @input-border-radius. The sense of consistency tells me it should be renamed to @input-border-radius-base. But the sense of backwards compatibility rings a bell.
Should I touch @input-border-radius or should I leave it, or should I add an alias?

@mdo
Copy link
Member

mdo commented Sep 5, 2014

Leave it as-is and simply add the new variables. We can revisit them all in v4.

@hoho
Copy link
Contributor Author

hoho commented Sep 5, 2014

Should I commit autogenerated files? It looks like someone else's uncommited changes appear there too.

@cvrebert
Copy link
Collaborator

cvrebert commented Sep 5, 2014

Should I commit autogenerated files?

  • *.min.css – No
  • *.css – Yes

@hoho
Copy link
Contributor Author

hoho commented Sep 5, 2014

modified:   dist/css/bootstrap.css.map
modified:   docs/_includes/customizer-variables.html
modified:   docs/assets/js/raw-files.min.js
modified:   docs/dist/css/bootstrap.css
modified:   docs/dist/css/bootstrap.css.map
modified:   docs/dist/css/bootstrap.min.css

.map, .html, .js?

My changes affect only dist/css/bootstrap.css.map, docs/assets/js/raw-files.min.js, docs/_includes/customizer-variables.html and docs/dist/css/bootstrap.css.map.

@cvrebert
Copy link
Collaborator

cvrebert commented Sep 5, 2014

Among those files, only docs/dist/css/bootstrap.css should be included.

@hoho
Copy link
Contributor Author

hoho commented Sep 5, 2014

#14542 — that's it.

mdo added a commit that referenced this issue Sep 8, 2014
Cleaned up fix for #14511 (input border-radii variables)
@mdo
Copy link
Member

mdo commented Sep 8, 2014

Fixed in #14566, which supersedes #14542 but keeps the original commit.

@mdo mdo closed this as completed Sep 8, 2014
@mdo mdo added this to the v3.2.1 milestone Sep 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants