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

Skatepark: apply wide width to header #4464

Merged
merged 2 commits into from
Aug 24, 2021
Merged

Skatepark: apply wide width to header #4464

merged 2 commits into from
Aug 24, 2021

Conversation

jffng
Copy link
Contributor

@jffng jffng commented Aug 24, 2021

Changes proposed in this Pull Request:

This PR adds some styles to ensure the Skatepark header does not extend beyond the wide width. Now that #4459 has merged, this is ready for a review.

Related issue(s):

Closes #4407

@jffng jffng marked this pull request as draft August 24, 2021 15:56
@jffng jffng added the [Theme] Skatepark Automatically generated label for Skatepark. label Aug 24, 2021
@jffng
Copy link
Contributor Author

jffng commented Aug 24, 2021

I tested this with #4448 or #4459, and both work :)

@jffng jffng force-pushed the fix/skatepark-header branch from 3d71ae6 to cfab582 Compare August 24, 2021 17:42
@jffng jffng requested review from a team and melchoyce August 24, 2021 17:43
@jffng jffng marked this pull request as ready for review August 24, 2021 17:43
Copy link
Contributor

@pbking pbking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK.
I think we could do the same with another group wrapping the header. I don't believe either is wrong but wrapping with a "full width" group would be "reproducable" in the template editor. With the state of flex controls (currently lacking) I don't think that matters now. Once flex is more powerful this will need to be changed again then and can be re-evaluated then.

@jffng
Copy link
Contributor Author

jffng commented Aug 24, 2021

I think we could do the same with another group wrapping the header.

I opened a PR to try that idea #4466, it works but yeah I'm not sure which one is preferred. Going to merge this one for now to get the design in place.

@jffng jffng merged commit 357f0ce into trunk Aug 24, 2021
@jffng jffng deleted the fix/skatepark-header branch August 24, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Theme] Skatepark Automatically generated label for Skatepark.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skatepark: Header should max out at the wide-width value
3 participants