Skip to content

Convert tooltip and popover's arrows to generated CSS content via ::before/::after #17238

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 10, 2016
Merged

Convert tooltip and popover's arrows to generated CSS content via ::before/::after #17238

merged 2 commits into from
Oct 10, 2016

Conversation

brnrdog
Copy link
Contributor

@brnrdog brnrdog commented Aug 23, 2015

This PR removes the div.tooltip-arrow and div.popover-arrow rendering the arrows using ::before and ::after pseudo elements.

image

image

@cvrebert
Copy link
Collaborator

And what about popover arrows?

bootstrap/scss/_popover.scss

Line 26 in cf7819d

.popover-arrow {

@brnrdog
Copy link
Contributor Author

brnrdog commented Aug 23, 2015 •
edited
Loading

Cool, I'm going to work in it.

@brnrdog brnrdog changed the title Convert tooltip's arrows to generated CSS content via :before Convert tooltip and popover's arrows to generated CSS content via :before Aug 23, 2015
@brnrdog brnrdog changed the title Convert tooltip and popover's arrows to generated CSS content via :before Convert tooltip and popover's arrows to generated CSS content via :before/:after Aug 23, 2015
@brnrdog
Copy link
Contributor Author

brnrdog commented Aug 23, 2015

Just added popover's arrows via :beforeand :after too.

@pedrofelipe
Copy link

👍

@brnrdog brnrdog changed the title Convert tooltip and popover's arrows to generated CSS content via :before/:after Convert tooltip and popover's arrows to generated CSS content via ::before/::after Aug 24, 2015
@hnrch02
Copy link
Collaborator

hnrch02 commented Aug 24, 2015

Huh, we apparently lost the feature of tooltips and their arrows adjusting their position as to not flow out of the viewport. CC @fat

cvrebert
cvrebert reviewed Aug 24, 2015
View reviewed changes
scss/_popover.scss
@@ -23,19 +23,22 @@
&.bs-tether-element-attached-bottom {
margin-top: -$popover-arrow-width;

.popover-arrow {
bottom: -$popover-arrow-outer-width;
&::before,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mdo prefers the single-colon form for some reason. Please adjust these accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cvrebert we implemented doubles today in a PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kkirsche Ah, I'm not watching #17052 and hadn't seen the subsequent comments where he gave in.
Nevermind then! 😄

@cvrebert
Copy link
Collaborator

@hnrch02 On the one hand that's not surprising since the existing positioning logic was trashed in favor of using Tether, but on the other hand I believe that's exactly the sort of thing that Tether is supposed to help us handle correctly. File a new bug for that?

@hnrch02 hnrch02 mentioned this pull request Aug 25, 2015
Closed
@hnrch02
Copy link
Collaborator

hnrch02 commented Aug 25, 2015

@cvrebert Filed #17297.

@brnrdog
Copy link
Contributor Author

brnrdog commented Aug 26, 2015

Rebase applied.

@brnrdog
Copy link
Contributor Author

brnrdog commented Sep 4, 2015

@cvrebert, is there anything else I can help to make this PR ready to merge?

@cvrebert
Copy link
Collaborator

cvrebert commented Sep 4, 2015

CC: @mdo

@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 4, 2015

I say we put this on hold until we figure out if we want to resolve #17297 or not. Just gonna be more work restoring it if we remove it now.

@mdo
Copy link
Member

mdo commented Nov 13, 2015

In favor of @hnrch02's comment.

@mdo mdo added the on-hold label Nov 13, 2015
@mdo mdo added this to the v4.0.0-alpha.5 milestone Oct 10, 2016
@mdo mdo removed the on-hold label Oct 10, 2016
@mdo mdo merged commit 8ef66df into twbs:v4-dev Oct 10, 2016
@mdo mdo mentioned this pull request Oct 10, 2016
Closed
@jnizet jnizet mentioned this pull request Oct 19, 2016
Closed
jnizet added a commit to jnizet/ng-bootstrap that referenced this pull request Oct 19, 2016
@jnizet
chore: update bootstrap to ^4.0.0-alpha.5
e0dacb9 
Closes ng-bootstrap#915

BREAKING CHANGE: ng-boostrap now uses bootstrap 4.0.0-alpha.5, which modified the markup used to generate tooltip and popover arrows (see twbs/bootstrap#17238). Using ng-bootstrap with an older version of bootstrap will thus not display tooltip and popover arrows anymore.
@jnizet jnizet mentioned this pull request Oct 19, 2016
Closed
@IdanCo IdanCo mentioned this pull request Apr 14, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
css v4
Projects
None yet
Milestone
v4.0.0-alpha.5
Development

Successfully merging this pull request may close these issues.
7 participants
@brnrdog @cvrebert @pedrofelipe @hnrch02 @mdo @kkirsche @krisdigital