Skip to content
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

Add more custom spacer utility classes #24

Open
Tracked by #7
dstrukt opened this issue Feb 6, 2022 · 6 comments
Open
Tracked by #7

Add more custom spacer utility classes #24

dstrukt opened this issue Feb 6, 2022 · 6 comments
Assignees

Comments

@dstrukt
Copy link
Member

dstrukt commented Feb 6, 2022

/* Custom space utils, as Bootstrap has a different spacing scale for > M */
.pt-l { padding-top: var(--btcpay-space-l); }
.pt-xl { padding-top: var(--btcpay-space-xl); }
.pb-l { padding-bottom: var(--btcpay-space-l); }
.pb-xl { padding-bottom: var(--btcpay-space-xl); }
.mt-l { margin-top: var(--btcpay-space-l); }  
.mt-xl { margin-top: var(--btcpay-space-xl); }
.mb-l { margin-bottom: var(--btcpay-space-l); }
.mb-xl { margin-bottom: var(--btcpay-space-xl); }

We should add one for (M)edium as well.

@dstrukt dstrukt changed the title Add more custom spacer utilities Add more custom spacer utility classes Feb 6, 2022
@dennisreimann
Copy link
Member

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.

@dstrukt
Copy link
Member Author

dstrukt commented Feb 6, 2022

I see, sounds good .. will leave this task open for tracking, and we'll reference and close when/if we address those spacings.

@dennisreimann
Copy link
Member

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.

@dstrukt
Copy link
Member Author

dstrukt commented Apr 7, 2022

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.

@dstrukt
Copy link
Member Author

dstrukt commented May 1, 2022

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?

@dennisreimann
Copy link
Member

This hasn't been done yet.

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

No branches or pull requests

2 participants