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.

Sign up for GitHub

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

Jump to bottom

Default margin and padding of headers is cancelled out #637

Open
sonniesedge opened this issue Dec 9, 2021 · 2 comments
Open

Default margin and padding of headers is cancelled out #637

sonniesedge opened this issue Dec 9, 2021 · 2 comments
Labels
stale this issue is more than 30 days old

Comments

@sonniesedge
Copy link

sonniesedge commented Dec 9, 2021 •
edited
Loading

The default margins and paddings defined earlier in the brand context are cancelled out by the following.

frontend-toolkits/context/brand-context/springernature/scss/40-base/layout.scss

Line 64 in a53fbc2

margin: 0;

Is that intentional @sturobson, or an oversight?

If it's intentional then it feels wrong, as in my project I have to go in again and set these values up later on.
@sturobson
Copy link
Contributor

Hi @sonniesedge.

The only change I've made in this file is moving the font-size on the <body> to use a Sass variable

I agree this feels wrong but I'm not sure why it has been authored this way. Good ol' git blame suggests it's been like this for two years (although I could be completely misreading it).

Having spent quite a bit of time working through the various Sass files in the front-end toolkits (and also some hairy stuff that relied on Ruby Sass) there's quite a few things I've noted that well do with some further thought and improvement.

Of course, that doesn't help you here.

I'm thinking (whilst on the sofa) that the quickest 'fix' that would resolve this for you and not set fireworks off further down the cascade would be to swap these two lines around in the enhanced.scss file.

Happy to do some digging in the morning and create the relevant PR for this.

🙏🖤

@sturobson
Copy link
Contributor

There has been no update on this issue for 30 days. It is now marked as "stale". If you think this issue is still relevant and needs attention then reply with an update. If there is no update in 14 days, this issue will be closed.

@sturobson sturobson added the stale this issue is more than 30 days old label Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stale this issue is more than 30 days old
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sonniesedge @sturobson