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 the portal API for rendering <style> elements #55

Closed
WorldSEnder opened this issue Dec 2, 2021 · 4 comments
Closed

Use the portal API for rendering <style> elements #55

WorldSEnder opened this issue Dec 2, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request yew This issue is Yew specific

Comments

@WorldSEnder
Copy link
Collaborator

Instead of mounting and unmounting the elements ourselves, I suppose it would make sense to use portals in yew to put them there.

A plan to address this is to slightly change the StyleManager api introducing a trait StyleHost that implements the behaviour of the manager, along the lines of

trait StyleHost: std::fmt::Debug {
    fn prefix(&self) -> Cow<'static, str>;
    fn get_registry(&self) -> Rc<RefCell<StyleRegistry>>;
    fn mount(&self, content: &StyleContent);
    fn unmount(&self, id: &StyleId);
}

The current impl of mounting elements by hand can be retained by putting it on the Builder or some other class, but the stylist::yew::ManagerProvider components would use an impl that renders the mounted elements (with a portal) to the place where they should be mounted at, e.g. something along the lines of

<ManagerProvider>
  <StyleHost target={target.or_else(document.head)}> // StyleHost creates the portal for its children
    { for mounted_styles }
  </StyleHost>
  <ContextProvider<StyleManager> ...>
    { for props.children }
  </ContextProvider<StyleManager>>
</ManagerProvider>

It might also make sense to have an InlineStyleHost that renders the style elements simple at the point in the DOM where it is mounted.

@WorldSEnder WorldSEnder added enhancement New feature or request yew This issue is Yew specific labels Dec 2, 2021
@WorldSEnder WorldSEnder self-assigned this Dec 2, 2021
@futursolo
Copy link
Owner

Hmm, will this cause flash of unstyled content?

The current methods guarantees styles being inserted before content, so no need to worry about this.

But I think with portals, styles will be added after the content hence is possible to cause flash of unstyled content.

@WorldSEnder
Copy link
Collaborator Author

Portals should still render their content immediately, but I will take this into consideration in my PR.

@futursolo
Copy link
Owner

futursolo commented Dec 3, 2021

I think the flow is:

  • RenderRunner::<Comp>::run (element with class is rendered)
  • UpdateRunner::<ManagerProvider>::run (provider realises style updates)
  • RenderRunner::<ManagerProvider>::run (style is rendered)

It may also degrade performance when the number of style tags is large as virtual dom now needs to diff every <style /> tag and its content every time a new style is added (VDiff::apply) where as current method just needs to append a style tag.

In addition, it seems like this also makes <ManagerProvider /> mandatory?

I personally do not see a big advantage of using portals, but if you still want to give it a go, feel free to do so, and I will see how it goes in the PR.

@WorldSEnder
Copy link
Collaborator Author

I've finally come around to benchmarking this, and it confirms what you've been worried about. The yew diff algorithm is a big hit to performance, hence I don't think it's worth going down this road further.

One thing that might be worth thinking about would be to have a StyleHost trait as mentioned in the first comment - but this could be done by popular demand and in a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request yew This issue is Yew specific
Projects
None yet
Development

No branches or pull requests

2 participants