Skip to content

Fix container issue on live page before it goes live live#2998

Merged
lvachon1 merged 6 commits intomainfrom
lev/fix/remove_preview_container
Mar 12, 2026
Merged

Fix container issue on live page before it goes live live#2998
lvachon1 merged 6 commits intomainfrom
lev/fix/remove_preview_container

Conversation

@lvachon1
Copy link
Contributor

@lvachon1 lvachon1 commented Mar 5, 2026

Scope

Asana Ticket: ⚽️ Fix container issue on live page before it goes live live

Implementation

Stopped the preview page from assuming it's children pages wanted a container wrapper, child pages will supply their own now. Removed now redundant param that disabled the container.

Screenshots

How to test

http://localhost:4001/preview/ - Everything looks the same

…ontainer wrapper, child pages will supply their own now. Removed now redundant param that disabled the container.
@lvachon1 lvachon1 requested a review from a team as a code owner March 5, 2026 20:14
@lvachon1 lvachon1 requested a review from joshlarson March 5, 2026 20:14
Copy link
Contributor

@joshlarson joshlarson left a comment

Choose a reason for hiding this comment

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

I think the scope here needs to be a bit wider - this PR will fix the container issue for live views that are still in preview, but not for live views that are out of preview mode and use the regular live layout, which also indiscriminately wraps its inner block in a container.

The solution that I'm thinking is...:

  • Update the live layout so that it no longer wraps its contents in <div class="container">...</div>.
    • (Possibly at this point, we just delete the live layout, because it's not doing anything. In theory, we could keep it, in case we ever decide that we need or want a shared live view layout that's separate from non-live-view pages, but that seems pretty unlikely to me)
  • Update the live views that currently use the live layout to wrap themselves in their own <div class="container">...</div>.

Copy link
Contributor

@joshlarson joshlarson left a comment

Choose a reason for hiding this comment

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

On further reflection, I think the changes I requested could very easily be done in a separate PR.

I'm not opposed to putting those changes into this PR, but also not opposed to doing them as a follow-up.

@lvachon1 lvachon1 requested a review from joshlarson March 11, 2026 18:19
@lvachon1
Copy link
Contributor Author

I removed the container from other pages before I saw your comment, Josh. I'm requesting a re-review just to make sure I haven't done anything stupid.

Copy link
Contributor

@joshlarson joshlarson left a comment

Choose a reason for hiding this comment

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

Amazing!

@lvachon1 lvachon1 enabled auto-merge (squash) March 12, 2026 16:57
@lvachon1 lvachon1 merged commit 97032bf into main Mar 12, 2026
17 checks passed
@lvachon1 lvachon1 deleted the lev/fix/remove_preview_container branch March 12, 2026 17:09
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.

2 participants