-
Notifications
You must be signed in to change notification settings - Fork 117
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
DEP Upgrade bootstrap and reactstrap #1308
DEP Upgrade bootstrap and reactstrap #1308
Conversation
7e7b26d
to
3d0c764
Compare
max-width: 900px; | ||
width: 900px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bootstrap styling changed to using width
instead of max-width
so this needed to be changed to match
// Fixes issue when rendering reactstrap tooltip | ||
// Warning: `NaN` is an invalid value for the `left` css style property. | ||
// https://stackoverflow.com/a/70157330 | ||
// Have refactored to not use an anonymous class with a static property being assigned | ||
// so that it passed eslint | ||
jest.mock('popper.js', () => { | ||
const PopperJS = jest.requireActual('popper.js'); | ||
return { | ||
__esModule: true, | ||
default: jest.fn(() => ({ | ||
placements: PopperJS.placements, | ||
destroy: () => {}, | ||
scheduleUpdate: () => {}, | ||
})), | ||
}; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer needed
.form-label { | ||
margin-bottom: 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Counteracts bootstrap adding a margin for this which broke the "display title?" checkbox styling.
See silverstripe/silverstripe-admin#1889 which links to the specific upgrade guide entries for most changes made.
Upgrade docs
Issue