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

Fullwidth page fix and theme options update. #197

Closed
wants to merge 3 commits into from

Conversation

johnnypeck
Copy link
Contributor

Fix the full page width issue, adds a theme option for the class and sets some sensible defaults for the various frameworks on that option.

@johnnypeck
Copy link
Contributor Author

I accidentally included a deactivation hook in roots-options.php which I had intended to remove. It wouldn't really hurt anything and may even alleviate a few headaches for some but it could just be superfluous. Let me know and I'll fix and re-pull if needed.

@swalkinshaw
Copy link
Member

Could you remove the deactivation hook? We should consider it separately if it has some benefits. Don't want that change to get lost in this one. Otherwise, pull request looks like a needed solution.

@johnnypeck
Copy link
Contributor Author

I removed the deactivation hook. It's really only useful to make sure options are rebuilt when a change is made during development. Cheers.

@retlehs
Copy link
Sponsor Member

retlehs commented Dec 29, 2011

thanks @johnnypeck - i would have pulled this in already but now i'm rethinking including the full width template. do you think it's worth it to keep it?

@johnnypeck
Copy link
Contributor Author

@retlehs I think it makes sense to have the full width. It's a very common use case and if anything shows an example full width layout to start with for a child theme to implement.

@retlehs
Copy link
Sponsor Member

retlehs commented Jan 2, 2012

one last thing: in the options the label is called '#fullwidth CSS Classes'

it's just a class on the #main ID so a better label might be something like 'Full Width CSS Classes'

once that's updated i'll pull this in, thx @johnnypeck

@johnnypeck
Copy link
Contributor Author

@retlehs No worries. Good call on the option naming. Have a good one.

@aboutaaron
Copy link

I went ahead and tried to implement this code and noticed this with 960.gs:

When I created a new page with the Full Width Template in 960.gs, I successfully saw the grid_12 class appear.

However, when I made it the static "Front Page" in the WordPress "Reading" settings, the sidebar div reappeared.

I figured it was a front-page.php deal and changed $roots-options to fullwidth_class in the file. This worked for my use with the theme, but perhaps there's a way to conditionally load this class if full-width is selected.

If I figure something out I'll add a pull request.

Thanks a lot @johnnypeck and @retlehs!

@johnnypeck
Copy link
Contributor Author

@aboutaaron WordPress automatically uses front-page.php if it is present in a theme when you select a static home page. If it was not present in roots then you could use any template you like.

@johnnypeck
Copy link
Contributor Author

@retlehs Any word on getting this pull merged?

retlehs added a commit that referenced this pull request Jan 8, 2012
@retlehs
Copy link
Sponsor Member

retlehs commented Jan 8, 2012

done with 9da422e, thx @johnnypeck

@retlehs retlehs closed this Jan 8, 2012
oxyc pushed a commit to generoi/sage that referenced this pull request May 10, 2019
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

Successfully merging this pull request may close these issues.

None yet

4 participants