Open dstrukt opened 2 years ago
The ones below L have the same values as the Bootstrap ones. There's no need to add them as that would introduce more ambiguity -- I'd even rather clean up the difference between our spacings and Bootstrap.
I see, sounds good .. will leave this task open for tracking, and we'll reference and close when/if we address those spacings.
Here's our scale:
Name | Value | Variable |
---|---|---|
XS | 4px | --btcpay-space-xs |
S | 8px | --btcpay-space-s |
M | 16px | --btcpay-space-m |
L | 32px | --btcpay-space-l |
XL | 64px | --btcpay-space-xl |
XXL | 80px | --btcpay-space-xxl |
Bootstrap spacing:
Name | Value | Pixel |
---|---|---|
1 | 0.25rem | 4px |
2 | 0.5rem | 8px |
3 | 1rem | 16px |
4 | 1.5rem | 24px |
5 | 3rem | 48px |
Their gap between 4
and 5
is quite big, that's why we are using our L
in some places (especially for bottom margins). On the other side our scale lacks the 1.5rem
value, which is why we are using the Bootstrap 4
oftentimes, also mostly for bottom margins.
We do not use our current XL
and XXL
variables as utility classes in the code, they are only used directly in the CSS for layout purposes.
Though we use Bootstraps spacing utility classes way more than ours, I think the best way forward is to extend our scale with the 24px/1.5rem
and 48px/3rem
values and convert to that new scale.
100% agree on adding 24px, I use that quite often, and would help with some of the more option-heavy settings screens, but for that specifically I will likely suggest we update the padding/margin for the .form-group
class to address this.
Additionally, while we're making these changes .. I might even make an argument for a 40px OR 48px, but 40px is probably best ... spacer as a step between 32 & 64 .. for more flexibility in the future.
Following up on this:
Though we use Bootstraps spacing utility classes way more than ours, I think the best way forward is to extend our scale with the 24px/1.5rem and 48px/3rem values and convert to that new scale.
If this was done / merged, shall I close this issue @dennisreimann?
This hasn't been done yet.
We should add one for (M)edium as well.