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

feat: render Preview inside iframe, inject built Tailwind CSS instead of runtime. #385

Merged
merged 7 commits into from
Jul 31, 2024

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Jul 29, 2024

TL;DR

Added a new build script for Tailwind CSS and introduced react-frame-component to the Studio application.

This was intended to allow for isolation of preview in regards to the rest of the application (able to set specific breakpoints, etc). 90% of the way there, but click events and scroll locks still trigger on root for some reason. Mitigated with an effect but might cause more inconvenience in the future. Should still be good enough.

TODO (In future PRs):

  • add turbo dependency to run tw-build script before dev/build/etc.

What changed?

  • Added a build script build:preview-tw in package.json to compile Tailwind CSS.
  • Introduced a new CSS file preview-tw.css generated by Tailwind CSS.
  • Added the react-frame-component package to dependencies.
  • Created a PreviewIframe component utilizing react-frame-component.
  • Integrated PreviewIframe into the page editor.
  • Restructured MobileNavMenu components in the Navbar.

How to test?

  1. Run npm install to install the new dependency react-frame-component.
  2. Execute npm run build:preview-tw to compile the Tailwind CSS file.
  3. Navigate to the page editor in the Studio application and confirm the preview is rendered inside an iframe with the correct styles.
  4. Verify the mobile navigation menu functions correctly.

Why make this change?

To improve the styling and component structure of the Studio application, and to provide a better development experience with Tailwind CSS and iframe previews.

Copy link

vercel bot commented Jul 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
isomer-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2024 6:33am

Copy link

linear bot commented Jul 29, 2024

@karrui karrui changed the title feat: add scripts to generate build-time tw-styles for injection [FEATURE]: add PreviewIframe and Tailwind CSS integration with build script Jul 29, 2024
@karrui karrui changed the title [FEATURE]: add PreviewIframe and Tailwind CSS integration with build script feat: render Preview inside iframe, inject built Tailwind CSS instead of runtime. Jul 29, 2024
@karrui karrui marked this pull request as ready for review July 29, 2024 09:43
@karrui karrui requested a review from a team as a code owner July 29, 2024 09:43
@karrui karrui force-pushed the update_layout_stories branch from 864a454 to 4a86916 Compare July 30, 2024 03:24
@karrui karrui force-pushed the karrui/isom-1291-move-preview-into-iframe branch from c700501 to 663da21 Compare July 30, 2024 03:24
Copy link
Contributor

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

lgtm, clarification regarding the iframe. i've also noticed (while testing on local) that clicking on the hamburger menu expands the entire nav bar out to the whole document (and obscures the view), which doesn't happen on uat

i don't think expanding the nav bar out is a blocker to merge, but i caveat that we should definitely fix this soon because it's functionally not what users expect, and leads to a less than ideal experience.

another point of difference, though, is that now our iframe uses the embedded document's width as the width to bound the inner element. this means that now, we display a hamburger menu (at least on my mac's screen) whereas previously we displayed the menu. i think functionally this is a win but it does mean that we also need to have a sliding drawer for users to actually see their site in the desktop view.

@seaerchin
Copy link
Contributor

cc @sehyunidaaa any opinions on sliding drawer ui/ux?

Copy link
Contributor Author

karrui commented Jul 31, 2024

Merge activity

  • Jul 31, 2:00 AM EDT: @karrui started a stack merge that includes this pull request via Graphite.
  • Jul 31, 2:08 AM EDT: Graphite couldn't merge this PR because it had conflicts with the trunk branch.

@karrui karrui force-pushed the update_layout_stories branch from 4a86916 to 1bc41c7 Compare July 31, 2024 06:01
Base automatically changed from update_layout_stories to main July 31, 2024 06:08
@karrui karrui merged commit 0f3468d into main Jul 31, 2024
19 checks passed
@karrui karrui deleted the karrui/isom-1291-move-preview-into-iframe branch July 31, 2024 06:38
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