Popup and tooltip: dimensions in rem and bug fix of arrow placement by NielsHolt · Pull Request #23820 · twbs/bootstrap

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

 $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

    .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


    .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


    .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


    .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

    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


    .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

    .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

    .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

  .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

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

+  $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

    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

Uh oh!

Conversation

NielsHolt commented Sep 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Johann-S commented Sep 4, 2017

Uh oh!

NielsHolt commented Sep 4, 2017

Uh oh!

NielsHolt commented Sep 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NielsHolt commented Sep 12, 2017

Uh oh!

wojtask9 commented Sep 13, 2017

Uh oh!

NielsHolt commented Sep 19, 2017

Uh oh!

mdo commented Oct 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NielsHolt commented Sep 3, 2017 •

edited

Loading

NielsHolt commented Sep 12, 2017 •

edited

Loading