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.

Sign up for GitHub

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

Jump to bottom

Is NamedViewFactory included automatically or isn't it? #538

Closed
rjrjr opened this issue Sep 2, 2021 · 4 comments · Fixed by #544
Closed

Is NamedViewFactory included automatically or isn't it? #538

rjrjr opened this issue Sep 2, 2021 · 4 comments · Fixed by #544
Labels
ui Related to UI integration
Milestone
post-v1.0.0

Comments

@rjrjr
Copy link
Contributor

rjrjr commented Sep 2, 2021

https://workflow-community.slack.com/archives/CHTFPR277/p1630599181038900?thread_ts=1630549583.028900&cid=CHTFPR277
@rjrjr rjrjr added this to the post-v1.0.0 milestone Sep 2, 2021
@rjrjr rjrjr added the ui Related to UI integration label Sep 2, 2021
@lucamtudor lucamtudor mentioned this issue Sep 2, 2021
Open
@rjrjr
Copy link
Contributor Author

rjrjr commented Sep 7, 2021

@lucamtudor Looks like WorkflowLayout#start is where we ensure that NamedViewFactory is in play. Were you using that?

@lucamtudor
Copy link

lucamtudor commented Sep 7, 2021 •
edited
Loading

@rjrjr I'm using the new workflow.renderAsState() (here)

@rjrjr
Copy link
Contributor Author

rjrjr commented Sep 7, 2021

Sweet. Okay, we should fix that. Noted.

@rjrjr
Copy link
Contributor Author

rjrjr commented Sep 7, 2021

WorkflowRendering is probably the right spot. And should probably move the existing line from WorkflowLayout to WorkflowViewStub.

rjrjr added a commit that referenced this issue Sep 7, 2021
@rjrjr
Exposes ViewEnvironment.withDefaults, makes WorkflowRendering() use it.
76707a5 
Ensures that `Named` works in compose based apps, and makes the need for
containers to do this kind of thing a bit more explicit.

Also moves the existing defaults swizzle from `WorkflowLayout` to
`WorkflowViewStub`, one step closer to getting rid of the former.

Fixes #538
@rjrjr rjrjr mentioned this issue Sep 7, 2021
Closed
rjrjr added a commit that referenced this issue Sep 7, 2021
@rjrjr
Builds in NamedViewFactory properly.
f6a50a5 
`ViewRegistry.defaultViewFactories` has never worked very well, and it
completely failed `WorkflowRendering()`. We get rid of it, and instead ensure
that `NamedViewFactory` is always available via the same hook that makes
`AndroidViewRendering` work.

Fixes #538
@rjrjr rjrjr mentioned this issue Sep 7, 2021
Merged
@rjrjr rjrjr closed this as completed in #544 Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ui Related to UI integration
Projects
None yet
Milestone
post-v1.0.0
2 participants
@rjrjr @lucamtudor