Skip to content

Fixes #30503 to replace renamed position utility #30505

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

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

mdo
Copy link
Member

@mdo mdo commented Apr 2, 2020

Fixes #30503

@mdo mdo requested a review from a team as a code owner April 2, 2020 22:36
@MartijnCuppens
Copy link
Member

The navbars still seems to be broken: https://deploy-preview-30505--twbs-bootstrap.netlify.com/docs/4.4/components/navbar/

I think we better use the .fixed-top class here? I also spotted some margin hacks, I think we can get rid of them too:

.bd-example {
.fixed-top,
.sticky-top {
position: static;
margin: -1rem -1rem 1rem;
}
.fixed-bottom {
position: static;
margin: 1rem -1rem -1rem;
}
@include media-breakpoint-up(sm) {
.fixed-top,
.sticky-top {
margin: -1.5rem -1.5rem 1rem;
}
.fixed-bottom {
margin: 1rem -1.5rem -1.5rem;
}
}
}

@XhmikosR XhmikosR linked an issue Apr 15, 2020 that may be closed by this pull request
@XhmikosR
Copy link
Member

@mdo friendly ping 🙂 BTW this doesn't seem to apply to master, right?

@mdo mdo force-pushed the v4-navbar-fixed-class branch from 72f9c79 to e307d64 Compare April 30, 2020 02:10
@mdo mdo force-pushed the v4-navbar-fixed-class branch from e307d64 to 7ac5e4f Compare April 30, 2020 02:13
@mdo
Copy link
Member Author

mdo commented Apr 30, 2020

Indeed! Updated to use .fixed-top and nuked the .position-fixed override styles.

@XhmikosR XhmikosR requested a review from MartijnCuppens April 30, 2020 05:44
Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

BTW this doesn't seem to apply to master, right?

Nope, pos-f-t doesn't seem to be present anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible error in documentation
3 participants