Fix usage of ltr/rtl padding for notifications (Fixes #3025)
This is a bugfix. Addresses #3025
Proposed solution
Unify the usage of ltr and rtl padding values behind one variable, $notification-padding.
Currently, $notification-padding is set, but isn't used at all. The real padding is written using +ltr / +rtl mixins, and using the specific values $notification-padding-ltr and $notification-padding-rtl.
This obscures what we really want to do: Offer a configurable value for padding, and set an appropriate default based on the value of $rtl.
To correct this, we create a 4th variable, $default-notification-padding, which is populated by @if $rtl, then set the default (replaceable) value of $notification-padding with this new variable.
Tradeoffs
- Add a new variable that isn't necessarily worth documenting, and makes this definition more verbose
- Add an insignificant amount of compute time during render
Testing Done
- [x] Pull the latest
masterbranch - [x] Make sure your Sass code is compliant with the Bulma Sass styleguide
- [x] Make sure your PR only affects
.sassor documentation files - [x] Try your changes.
The following code blocks are pairs of (test case bulma.sass, and the output of grep -A4 "\.notification {" css/bulma.css after running npm run build):
@charset "utf-8"
/*! bulma.io v0.9.4 | MIT License | github.com/jgthms/bulma */
@import "sass/utilities/_all"
@import "sass/elements/notification.sass"
.notification {
background-color: whitesmoke;
border-radius: 4px;
position: relative;
padding: 1.25rem 2.5rem 1.25rem 1.5rem;
Here we see the default still stays ready for ltr with a delete button on the right.
@charset "utf-8"
/*! bulma.io v0.9.4 | MIT License | github.com/jgthms/bulma */
@import "sass/utilities/_all"
$rtl: true
@import "sass/elements/notification.sass"
.notification {
background-color: whitesmoke;
border-radius: 4px;
position: relative;
padding: 1.25rem 1.5rem 1.25rem 2.5rem;
When $rtl is enabled, the padding is now updated without any other changes.
@charset "utf-8"
/*! bulma.io v0.9.4 | MIT License | github.com/jgthms/bulma */
@import "sass/utilities/_all"
$rtl: true
$notification-padding: 0px
@import "sass/elements/notification.sass"
.notification {
background-color: whitesmoke;
border-radius: 4px;
position: relative;
padding: 0px;
Now even with $rtl enabled, the output is still the specified value of $notification-padding.
Changelog updated?
No.
I don't see how this is an improvement? It's simply putting the @if $rtl logic in the variable definition, rather than in the property definition?
One thing I agree on is to remove $notification-padding which isn't used anymore.
It re-enables $notification-padding and makes its default behave sensibly, but that's only an improvement if that is the goal (which is my preference).
If your preference is to remove $notification-padding, I suppose it's not an improvement. If so, I can change this PR to simply remove it from the SASS and the documentation. The real upstream problem is that I was misled by the documentation and either of these options fixes that.