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 rows for child slots #525

Merged
merged 10 commits into from Jun 6, 2018

Conversation

6 participants
@natefaubion
Copy link
Collaborator

natefaubion commented Apr 24, 2018

Fixes #439

This is still a WIP but is usable and ready for testing/review of the approach. This also fixes the issue where you can't rebind the output handler once the component is mounted. It should just work like normal handlers do.

I think we should remove the Component/Parent distinction for HTML and DSL now. The only difference now will be that leaf components/html just have () as their slot type. We can also provide absurdSlot :: forall a b. ComponentSlot () a -> b

  • Add back queryAll support
  • Remove Component/Parent distinction
  • Fix all examples
  • Fix documentation comments
  • Remove ChildPath/Prism
@safareli

This comment has been minimized.

Copy link
Contributor

safareli commented Apr 24, 2018

Looked at examples and it's great 👏

natefaubion added some commits Apr 25, 2018

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 25, 2018

I wonder if it's worth baking ComponentSlot into HTML.

@thomashoneyman

This comment has been minimized.

Copy link
Contributor

thomashoneyman commented Apr 25, 2018

👍 on removing the parent / component distinction and type synonyms. This is much clearer IMO.

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 25, 2018

Also, LifecycleComponentSpec is just Component' now, and lifecycleComponent is just mkComponent. Not sure how we want to handle that now.

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 25, 2018

If we can find a good solution to the lifecycle situation, we can just have the one constructor. That would be ideal.

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Apr 25, 2018

If nothing else, we could change the API so it's like #517 - it wasn't what you meant with the proposal for it, but doing would reduce the lifecycle handling aspects to one function at least.

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 25, 2018

I think for now I'm going to remove the distinction and just have component, and we can address lifecycle stuff in another PR.

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 26, 2018

I think most of the example diffs are due to windows line endings :rage4:

@natefaubion natefaubion changed the title WIP: Use rows for child slots Use rows for child slots Apr 26, 2018

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 26, 2018

I think this is ready for review.

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 26, 2018

One thing that's neat about the rows approach is you can export a default slot constructor from your component module.

From the components-inputs example, Display could export:

display :: forall p f m r. Ord p => p -> Input -> H.ComponentHTML f (display :: Slot p | r) m
display slot i = HH.slot (SProxy :: SProxy "display") slot component i absurd

Which can be used in the Container just as:

  render :: State -> H.ComponentHTML Query ChildSlots m
  render state =
    HH.div_
      [ HH.ul_
          [ Display.display 1 state
          , Display.display 2 (state * 2)
          , Display.display 3 (state * 3)
          , Display.display 4 (state * 10)
          , Display.display 5 (state * state)
          ]
  ...
@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented Apr 26, 2018

Bikeshed opportunity: order of type arguments.

@paulyoung

This comment has been minimized.

Copy link

paulyoung commented May 11, 2018

This is amazing 👏

@acple

This comment has been minimized.

Copy link
Contributor

acple commented May 11, 2018

(re-)rendering child components always output "Duplicate slot address" warning. cond inverted?

@natefaubion

This comment has been minimized.

Copy link
Collaborator

natefaubion commented May 12, 2018

(re-)rendering child components always output "Duplicate slot address" warning. cond inverted?

Yes, you're right. Thanks!

@paulyoung

This comment has been minimized.

Copy link

paulyoung commented May 19, 2018

Is this likely to land any time soon?

@garyb

This comment has been minimized.

Copy link
Member

garyb commented May 19, 2018

After 0.12 - I'll make a release with the minimal necessary updates for 0.12, and then shortly after a release with things like this and the reworked subscriptions.

@cryogenian cryogenian referenced this pull request May 28, 2018

Merged

Consolidated prs #532

@garyb garyb merged commit 409cd2d into slamdata:master Jun 6, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment