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

Add Surface.Components.Dynamic.LiveComponent #518

Merged
merged 7 commits into from Nov 10, 2021
Merged

Add Surface.Components.Dynamic.LiveComponent #518

merged 7 commits into from Nov 10, 2021

Conversation

tiberiuc
Copy link
Contributor

@tiberiuc tiberiuc commented Nov 3, 2021 •
edited

This PR is related to issue #516 and #519

  • added new component to support dynamic stateful components

This was referenced Nov 4, 2021
Closed
Closed
Malian
Malian approved these changes Nov 6, 2021
View reviewed changes
Copy link
Collaborator

@Malian Malian left a comment

Choose a reason for hiding this comment

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

Except the minor changes, LGTM.

@msaraiva I do not fully understand the ins and outs of having dynamics components implemented like this. I think it's safest if you take a look at it ;)

test/surface/components/dynamic/live_component_test.exs Outdated Show resolved Hide resolved
lib/surface/components/dynamic/live_component.ex Outdated Show resolved Hide resolved
test/surface/components/dynamic/live_component_test.exs Outdated Show resolved Hide resolved
@Malian Malian requested a review from msaraiva November 6, 2021 15:27
msaraiva
msaraiva reviewed Nov 9, 2021
View reviewed changes
Copy link
Collaborator

@msaraiva msaraiva left a comment

Choose a reason for hiding this comment

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

Hi @tiberiuc!

Thank you for the PR. Great work! I left just a couple of comments for your consideration.

lib/surface/components/dynamic/live_component.ex Outdated Show resolved Hide resolved
test/surface/components/dynamic/live_component_test.exs Outdated Show resolved Hide resolved
@tiberiuc
removed unecessary tests, fix default slot for Dynamic.LiveComponent,…
62c3949 
… added tests for default slot on Dynamic.LiveComponent
@Malian Malian merged commit d1c6b9e into surface-ui:master Nov 10, 2021
@Malian
Copy link
Collaborator

Malian commented Nov 10, 2021

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msaraiva msaraiva Awaiting requested review from msaraiva

@Malian Malian Awaiting requested review from Malian

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tiberiuc @Malian @msaraiva