Skip to content

Custom select validation tweaks#27175

Merged
XhmikosR merged 3 commits intotwbs:v4-devfrom
MartijnCuppens:custom-select-validation-padding
Oct 23, 2018
Merged

Custom select validation tweaks#27175
XhmikosR merged 3 commits intotwbs:v4-devfrom
MartijnCuppens:custom-select-validation-padding

Conversation

@MartijnCuppens
Copy link
Copy Markdown
Member

The padding of custom selects doesn't cover the background, if the text in the select box is to long, the text and background image will overlap.

This PR also removes unneeded calc() functions.

Before:
image

After:
image

Comment thread scss/mixins/_forms.scss Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we take the opportunity to clean this up a bit here and turn these major values into variables? Separately, was the / working as intended before? Curious if Sass was trying to compute this vs keeping it as a property separator.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A calc() function was used, that's why the / worked before.

Do you want to use the variables in the _variables.scss file or declare them here in the _forms.scss?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, yeah, got it. Most likely in _variables.scss.

@XhmikosR
Copy link
Copy Markdown
Member

@MartijnCuppens: can you move the variable to _variables.scss?

@mdo
Copy link
Copy Markdown
Member

mdo commented Oct 21, 2018

Tests are failing for linter issues.

@MartijnCuppens MartijnCuppens force-pushed the custom-select-validation-padding branch from 6896add to 3693246 Compare October 22, 2018 18:38
@MartijnCuppens
Copy link
Copy Markdown
Member Author

I've moved some values to the variables file as mdo suggested and simplified the if/else construction. Also fixed the linting issue and squashed everything into one commit.

@mdo mdo mentioned this pull request Oct 22, 2018
@XhmikosR XhmikosR merged commit dd53909 into twbs:v4-dev Oct 23, 2018
@MartijnCuppens MartijnCuppens deleted the custom-select-validation-padding branch October 23, 2018 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants