Skip to content

feat: add support for iframe and differing owner documents #38

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

Merged
merged 2 commits into from
Dec 8, 2021

Conversation

jquense
Copy link
Member

@jquense jquense commented Dec 7, 2021

This does a few things:

  • Add a useWindow hook for accessing the current window object, which can be set in unique situations, such as rendering components into an iframe. Defaults the the global window if available
  • Update references to global document, or window objects to use useWindow or if an element is present, their owner document/view

@jquense jquense requested a review from kyletsang December 7, 2021 03:06
Copy link
Collaborator

@kyletsang kyletsang left a comment

Choose a reason for hiding this comment

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

LGTM. Added couple comments

private state!: ContainerState;
protected state!: ContainerState;

protected ownerDocument: any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
protected ownerDocument: any;
protected ownerDocument: Document | undefined;

src/useWindow.ts Outdated

const Context = createContext(canUseDOM ? window : undefined);

export const Provider = Context.Provider;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should Provider be renamed to WindowProvider so consumers can avoid the import alias like in the example below?

@jquense jquense merged commit e4e7170 into main Dec 8, 2021
@jquense jquense deleted the window-context branch December 8, 2021 15:15
Brendan-csel added a commit to solid-libs/solid-bootstrap that referenced this pull request Dec 20, 2021
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