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

@max-page-width variable is ignored for .page element #493

Closed
swashbuck opened this issue Feb 6, 2024 · 7 comments · Fixed by #494
Closed

@max-page-width variable is ignored for .page element #493

swashbuck opened this issue Feb 6, 2024 · 7 comments · Fixed by #494
Assignees

Comments

@swashbuck
Copy link
Contributor

swashbuck commented Feb 6, 2024

Subject of the issue

The max-width of .page should be using the @max-page-width value. Instead, the Vanilla theme is overriding this with a max-width: inherit style.

Is this intentional? If not, should we remove the max-width: inherit style?

max-width-page

Your environment

  • Vanilla 9.14.1
  • FW 5.35.0

To reproduce

In your theme's CSS:

@body-background-color: #96D5E4;

.page {
  background-color: @white;
}

Expected results

Blue body background color is visible on the sides.

Actual results

Entire page is white because .page is the full window width and does not use @max-page-width.

@swashbuck
Copy link
Contributor Author

@guywillis Any thoughts on this?

@swashbuck swashbuck changed the title .page @max-page-width being ignored for max-width @max-page-width variable is ignored for .page element Feb 6, 2024
@guywillis
Copy link
Contributor

@swashbuck The preference at time of implementation was to plumb the container into core to match the rest of core Adapt but have the override in Vanilla so the visual impression is that Adapt goes to the edge of the screen but that it could be reverted with a small deletion of CSS.

@swashbuck
Copy link
Contributor Author

@guywillis Would it make sense to remove the max-width: inherit style from the theme and set the default value of @max-page-width to 100%? That would be simpler to customize since only one variable would need to be changed rather than having to find the offending style.

@guywillis
Copy link
Contributor

Makes a lot of sense to me

@swashbuck
Copy link
Contributor Author

Thanks, @guywillis . I've created PR #494 for this.

Question: do we still need the @max-width variable? From what I can tell, @max-width is only used to assign a value to @max-page-width. So, we could get by with only having @max-page-width. If we want to change it, I can create a separate ticket in Vanilla and the Core plugin.

@guywillis
Copy link
Contributor

guywillis commented Feb 8, 2024

I believe it is in to support those updating from older versions of the theme - backward compatibility.

@github-project-automation github-project-automation bot moved this from Needs Reviewing to Recently Released in adapt_framework: The TODO Board Feb 9, 2024
Copy link

github-actions bot commented Feb 9, 2024

🎉 This issue has been resolved in version 9.15.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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