Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

update for halogen1.0.0 #20

Merged
merged 1 commit into from
Feb 7, 2017
Merged

Conversation

cryogenian
Copy link
Member

@cryogenian cryogenian requested a review from garyb February 6, 2017 23:15
Copy link
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

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

Would like to see what you think about that dimensions thing before merging. The rest looks good though!

I think the package.json will need updating with an -- for the psa arguments too, Travis is failing before that even due to the halogen dependency mismatch.

@@ -65,12 +67,10 @@ render state =
where
renderOne ix =
HH.div
[ HP.key ("echarts-" <> show ix) ]
[ ]
Copy link
Member

Choose a reason for hiding this comment

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

Minorest of nitpicks: could use div_ here 😄

∷ ∀ eff g i r
. (MonadAff (EChartsEffects eff) g)
⇒ (i → Maybe (EChartsQuery Unit))
→ { width ∷ Int, height ∷ Int | r }
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should make the dimensions the input value for the component? Seems like a prime candidate for where that kind of thing would be great. The parameterised input thing here is pretty cool, but I think dimensions is by far the most useful possible thing we could use i for (it was actually the motivating case for why I added inputs to this release) - manual querying seems more appropriate for updating with new data, etc.

I guess we could do a mixture of the two, if we used Tuple Dimensions i -> ... perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@cryogenian cryogenian force-pushed the halogen-1.0.0 branch 2 times, most recently from 8739583 to 93c3e25 Compare February 7, 2017 13:33
@cryogenian
Copy link
Member Author

  • Removed redundant [ ]
  • Changed input type from i to Dimensions /\ i

@garyb garyb merged commit 630db38 into slamdata:master Feb 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants