Popup and tooltip: dimensions in rem and bug fix of arrow placement #23820

NielsHolt · 2017-09-03T22:11:10Z

Hi
This PR will change the dimensions of popover and tooltip from px to rem AND fix a bug in the way the arrow of popover and tooltips are placed. The current placement of the arrow is only centered on the element when border-width and arrow-width are default values. This PR will allow both border-width and arrow-width to be changed
It need to be merged/integrated with #23763
Could solve #23793 and issues in #23468
Setting $border-width: 20px; $popover-border-width:10px; in Bootstrap 4-beta would give

In this PR (with $popover-arrow-width: 1rem;) it would be

Allows for arrow-width in rem

… popup-in-rem # Conflicts: # scss/_variables.scss

Fix error on arrow margin to center arrow on element

houndci-bot · 2017-09-03T22:11:16Z

scss/_variables.scss

-$popover-arrow-width:                 10px !default;
-$popover-arrow-height:                5px !default;
+$popover-arrow-width:                 .8rem !default; 
+$popover-arrow-height:                .4rem !default; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:11:16Z

scss/_variables.scss


-$popover-arrow-width:                 10px !default;
-$popover-arrow-height:                5px !default;
+$popover-arrow-width:                 .8rem !default; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:11:16Z

scss/_variables.scss

-$popover-body-padding-y:           9px !default;
-$popover-body-padding-x:           14px !default;
+$popover-body-padding-y:           0.5625rem !default;
+$popover-body-padding-x:           0.875rem !default;


0.875 should be written without a leading zero as .875

houndci-bot · 2017-09-03T22:11:16Z

scss/_variables.scss


 $popover-body-color:               $body-color !default;
-$popover-body-padding-y:           9px !default;
-$popover-body-padding-x:           14px !default;
+$popover-body-padding-y:           0.5625rem !default;


0.5625 should be written without a leading zero as .5625

houndci-bot · 2017-09-03T22:11:16Z

scss/_variables.scss

-$popover-header-padding-y:             8px !default;
-$popover-header-padding-x:             14px !default;
+$popover-header-padding-y:             0.5rem !default;
+$popover-header-padding-x:             0.875rem !default;


0.875 should be written without a leading zero as .875

houndci-bot · 2017-09-03T22:11:17Z

scss/_variables.scss

@@ -608,40 +608,39 @@ $tooltip-max-width:           200px !default;
 $tooltip-color:               $white !default;
 $tooltip-bg:                  $black !default;
 $tooltip-opacity:             .9 !default;
-$tooltip-padding-y:           3px !default;
-$tooltip-padding-x:           8px !default;
+$tooltip-padding-y:           0.1875rem !default;


0.1875 should be written without a leading zero as .1875

houndci-bot · 2017-09-03T22:11:17Z

scss/_tooltip.scss

@@ -68,7 +71,7 @@

    .arrow::before {
      right: 0;
-      margin-top: -($tooltip-arrow-width - 2);
+      margin-top: $tooltip-arrow-margin; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:11:17Z

scss/_tooltip.scss

@@ -54,7 +57,7 @@
    }

    .arrow::before {
-      margin-left: -($tooltip-arrow-width - 2);
+      margin-left: $tooltip-arrow-margin; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:11:17Z

scss/_tooltip.scss

@@ -41,7 +44,7 @@
    }

    .arrow::before {
-      margin-top: -($tooltip-arrow-width - 2);
+      margin-top: $tooltip-arrow-margin; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:11:17Z

scss/_tooltip.scss

@@ -28,7 +31,7 @@
    }

    .arrow::before {
-      margin-left: -($tooltip-arrow-width - 2);
+      margin-left: $tooltip-arrow-margin; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:52Z

scss/_tooltip.scss

@@ -21,14 +22,16 @@
    height: $tooltip-arrow-height;
  }

+  $tooltip-arrow-margin: -$tooltip-arrow-width/2;


$tooltip-arrow-width/2 should be written with a single space on each side of the operator: $tooltip-arrow-width / 2

houndci-bot · 2017-09-03T22:17:52Z

scss/_popover.scss

      border-bottom-color: $popover-arrow-outer-color;
    }

    .arrow::after {
-      top: -($popover-arrow-outer-width - 1);
+      top: $popover-arrow-after-coor; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:52Z

scss/_popover.scss

@@ -106,17 +109,17 @@

    .arrow::before,
    .arrow::after {
-      margin-left: -($popover-arrow-width - 3);
+      margin-left: $popover-arrow-margin; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:52Z

scss/_popover.scss

      border-top-color: $popover-arrow-outer-color;
    }

    .arrow::after {
-      bottom: -($popover-arrow-outer-width - 1);
-      margin-left: -($popover-arrow-outer-width - 5);
+      bottom: $popover-arrow-after-coor; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:53Z

scss/_popover.scss

    }

    .arrow::before {
-      bottom: -$popover-arrow-outer-width;
-      margin-left: -($popover-arrow-outer-width - 5);
+      bottom: $popover-arrow-before-coor; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:53Z

scss/_popover.scss

@@ -39,17 +43,17 @@

  .arrow::before {
    content: "";
-    border-width: $popover-arrow-outer-width;
+    border-width: $popover-arrow-width; 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:53Z

scss/_popover.scss

  }

+  $popover-arrow-margin: -$popover-arrow-width/2; 
+  $popover-arrow-before-coor: calc(-#{$popover-arrow-width} - #{$popover-border-width});
+  $popover-arrow-after-coor : -$popover-arrow-width;


Variable names should be followed immediately by a colon

houndci-bot · 2017-09-03T22:17:53Z

scss/_popover.scss

  }

+  $popover-arrow-margin: -$popover-arrow-width/2; 


Line contains trailing whitespace
$popover-arrow-width/2 should be written with a single space on each side of the operator: $popover-arrow-width / 2

houndci-bot · 2017-09-03T22:17:53Z

scss/_popover.scss

-    width: $popover-arrow-width;
-    height: $popover-arrow-height;
+    width: calc(2*#{$popover-border-width} + #{$popover-arrow-width}); 
+    height: calc(2*#{$popover-border-width} + #{$popover-arrow-height}); 


Line contains trailing whitespace

houndci-bot · 2017-09-03T22:17:53Z

scss/_popover.scss

@@ -25,10 +25,14 @@
  .arrow {
    position: absolute;
    display: block;
-    width: $popover-arrow-width;
-    height: $popover-arrow-height;
+    width: calc(2*#{$popover-border-width} + #{$popover-arrow-width}); 


Line contains trailing whitespace

… popup-in-rem

Johann-S · 2017-09-04T08:02:14Z

is it possible to have a live example on a Codepen ?

NielsHolt · 2017-09-04T09:17:40Z

@Johann-S - I put some images in my original comment. I haven't used CodePen before but I give it a try (but it may take a while)

NielsHolt · 2017-09-12T08:05:29Z

Hi @Johann-S
I have made a Codepen here
It seems that floating-ui/floating-ui#417 and #23846 solves some of the issues, but I don't have the full overview

NielsHolt · 2017-09-12T08:27:04Z

Sorry - closed by mistake

wojtask9 · 2017-09-13T11:22:11Z

@NielsHolt
I created PR #23936 (based on your) but should fix some other issues.
Can you check if my PR fix also your issues?

NielsHolt · 2017-09-19T12:31:09Z

Ups - was closed by mistake. Reopened with updated demo

mdo · 2017-10-02T04:52:14Z

Not a fan of this approach, thanks though!

NielsHolt added 6 commits September 1, 2017 16:51

Test

06b37c2

Fix bug in placing popover and its arrow

dbb4d39

Allows for arrow-width in rem

Fix bug in placing popover and its arrow

851a3ec

Allows for arrow-width in rem

Merge branch 'popup-in-rem' of https://github.com/FCOO/bootstrap into…

1fa7116

… popup-in-rem # Conflicts: # scss/_variables.scss

Change sign of scss-variable

8d01e06

Change dimension of tooltip from px to rem

e33bb6e

Fix error on arrow margin to center arrow on element

houndci-bot reviewed Sep 3, 2017

View reviewed changes

0.XXrem -> .XXrem

cafb8d7

houndci-bot reviewed Sep 3, 2017

View reviewed changes

NielsHolt added 4 commits September 4, 2017 00:20

Typo bugs

f8c465c

Merge branch 'v4-dev' into popup-in-rem

38dbdd8

Typo bugs

db2f2db

Merge branch 'popup-in-rem' of https://github.com/FCOO/bootstrap into…

e22d0a5

… popup-in-rem

Johann-S added css v4 labels Sep 4, 2017

NielsHolt mentioned this pull request Sep 4, 2017

Convert px values to rem on tooltip variables #23468

Merged

alecpl mentioned this pull request Sep 7, 2017

Arrow out of popover #23846

Closed

NielsHolt closed this Sep 12, 2017

NielsHolt reopened this Sep 12, 2017

NielsHolt mentioned this pull request Sep 13, 2017

fix popover arrow computations #23110

Closed

wojtask9 mentioned this pull request Sep 13, 2017

fix popover arrow computations and .arrow position #23936

Closed

mdo closed this Oct 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Popup and tooltip: dimensions in rem and bug fix of arrow placement #23820

Popup and tooltip: dimensions in rem and bug fix of arrow placement #23820

NielsHolt commented Sep 3, 2017 •

edited

Loading

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

houndci-bot Sep 3, 2017

Johann-S commented Sep 4, 2017

NielsHolt commented Sep 4, 2017

NielsHolt commented Sep 12, 2017 •

edited

Loading

NielsHolt commented Sep 12, 2017

wojtask9 commented Sep 13, 2017

NielsHolt commented Sep 19, 2017

mdo commented Oct 2, 2017

Popup and tooltip: dimensions in rem and bug fix of arrow placement #23820

Popup and tooltip: dimensions in rem and bug fix of arrow placement #23820

Conversation

NielsHolt commented Sep 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Johann-S commented Sep 4, 2017

NielsHolt commented Sep 4, 2017

NielsHolt commented Sep 12, 2017 • edited Loading

NielsHolt commented Sep 12, 2017

wojtask9 commented Sep 13, 2017

NielsHolt commented Sep 19, 2017

mdo commented Oct 2, 2017

NielsHolt commented Sep 3, 2017 •

edited

Loading

NielsHolt commented Sep 12, 2017 •

edited

Loading