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

Use Shadow DOM for the CreateGroupForm app #8793

Merged
merged 3 commits into from
Jul 5, 2024
Merged

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Jul 4, 2024

  1. Move the Preact app for the new (feature-flagged) create-group-form page into a shadow DOM so the CSS of the surrounding site doesn't affect the Preact app and vice-versa.
  2. Remove the legacy CSS classes from the Preact app's HTML since they no longer do anything. Also simplify the HTML a bit, no longer mimicking the HTML structure of the legacy page. We may end up adding back some of this structure or something similar as we style the page in future PRs.
  3. Use frontend-shared's <Input>, <Textarea> and <Button> components in the new form
  4. Move the Tailwind/frontend-shared CSS into the shadow DOM, so that it applies to the Preact app and not to the surrounding site. This requires the Jinja templates to render the CSS URLs into a js-config object, and the Preact app to read the CSS URLs from this js-config object and render them into <style>'s within the shadow DOM.

This results in a form that's largely unstyled, apart from the styling that frontend-shared applies to the <input>, <textarea> and <button>. The form does also acquire a max-width, centered positioning, padding, etc because it's contained within the surrounding pages layout divs. The next PR's job will be to add Tailwind-style CSS to make the form once again look more like the legacy form, replacing the legacy styling that has been lost by the move into a shadow DOM.

image

@seanh seanh requested a review from robertknight July 4, 2024 14:34
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

So that the surrounding page's JavaScript and CSS can't affect the Preact app, and
vice-versa.

Shadow DOM only affects DOM queries and styling. The JS environment is still the same one. This means that eg. monkey-patching JS somewhere in the page would affect both parts. That's not a concern here though since we only need style isolation.

h/static/scripts/group-forms/index.tsx Outdated Show resolved Hide resolved
seanh added 3 commits July 5, 2024 13:02
So that the surrounding page's JavaScript and CSS can't affect the
Preact app, and vice-versa.
Use components from our component library (specifically `<Input>`,
`<Textarea>` and `<Button>`) in the `<CreateGroupForm>`.

Instead of using the HTML from h's legacy forms. This legacy HTML
doesn't work anymore anyway because the CSS it depends on is now
isolated from the Preact app by shadow DOM.
Since the Preact app has now been moved into a shadow DOM the app's CSS
was no longer working: the `<link>` elements for the app's stylesheets
were outside of the shadow DOM where they don't affect the content
inside the shadow DOM.

The `<link>`'s to the app's stylesheets need to be moved into the shadow
DOM. The problem is that the Preact app renders the contents of the
shadow DOM, but the stylesheet URLs are only available to the Jinja
templates and not available to the Preact app, but the Jinja templates
only render the outer page not the contents of the shadow DOM.

Get around this by having the Jinja templates pass the stylesheet URLs
into the Preact app so that the Preact app can render the `<link>`'s in
the shadow DOM: change the `create.html.jinja2` template to render the
app's stylesheet URLs in a `"js-config"` object instead of actually
rendering them as stylesheet `<link>`'s, then change the Preact app to
read the stylesheet URLs from the `"js-config"` object and to render
`<link>`'s for the stylesheet URLs into the shadow DOM.

This `"js-config"` object can be used more in future when we need the
Python code/Jinja templates to pass more configuration into the Preact
app.
@seanh seanh force-pushed the use-shadow-dom-for-preact branch from 70a053e to 99ecf14 Compare July 5, 2024 12:02
@seanh seanh marked this pull request as draft July 5, 2024 12:04
h/static/scripts/group-forms/index.tsx Dismissed Show dismissed Hide dismissed
@seanh seanh marked this pull request as ready for review July 5, 2024 13:04
@seanh seanh requested a review from robertknight July 5, 2024 13:04
@seanh seanh merged commit 1d72aac into main Jul 5, 2024
9 checks passed
@seanh seanh deleted the use-shadow-dom-for-preact branch July 5, 2024 17:01
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

2 participants