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

fix(select): change arrow trigger position to absolute #60

Merged
merged 7 commits into from
Jul 1, 2019

Conversation

efux
Copy link
Contributor

@efux efux commented Jun 26, 2019

This PR changes the placement of the arrow trigger in the select component from relative to absolute. This is similar to the arrows for the datepicker component. The reason for this change is, that otherwise the icon is higher than a normal field in the business library and therefore form elements are of different height when placed next to each other.

This change affects the public and business library.

efux added 5 commits June 26, 2019 10:24
The global stylesheet included in normal applications has the property box-sizing set. The tests do not have those elements. Therefore because the arrow has now position absolution the values have to be adapted.
}

@include mq($from: desktop5k) {
padding-right: 92px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Usually the desktop5k variant is simply calculated with the $scalingFactor5k. Why is this not the case here?

width: toPx($selectArrowWidthHeight * $scalingFactor4k);
height: toPx($selectArrowWidthHeight * $scalingFactor4k);
}

@include mq($from: desktop5k) {
right: 28px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as for padding-right.

cursor: pointer;
display: flex;
align-items: center;
padding-right: $selectTriggerPaddingRight;
padding-right: toPx($selectTriggerPaddingRight);
Copy link
Collaborator

Choose a reason for hiding this comment

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

padding-right should also be applied for native select.

@@ -78,11 +80,13 @@
@include publicOnly() {
background: url($selectNativeBackground) 97.5% no-repeat;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The padding-right change displaced the position of the arrow.
This can be solved by removing the percentage from background (and moving it ouside publicOnly/businessOnly) and adding background-position: center right $selectArrowRight (also replace background-position-x from the 4k/5k with background-position: toPx($selectArrowRight * $scalingFactor4k/5k))

@kyubisation
Copy link
Collaborator

LGTM. Thanks again. 😃

@kyubisation kyubisation merged commit 92512c2 into master Jul 1, 2019
@kyubisation kyubisation deleted the feature/business-select-arrows branch July 1, 2019 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants