search

InFact-coop / rsbc

https://rsbcapp.surge.sh/
MIT License
0 stars 1 forks source link

QA review end of day 1 #17

Closed astroash closed 7 years ago

astroash commented 7 years ago

https://github.com/InFact-coop/rsbc/blob/cd538a21f76369658fdd9ba08e8e1df8011b6402/src/elm/Components/Navbar.elm#L21-L45

@andrewMacmurray & @ivanmauricio I would love to get your opinion on mapping over touples to create elements. I often find my touples get out of hand with 3+ values. Should I be using records instead? Or is there another magical way I am missing...

Also all other tips super appreciated ❤️

andrewMacmurray commented 7 years ago

@astroash your intuition is right, if they all definitely need 3 parameters I'd use a record (you could call it something like NavItemConfig). If you find you need more flexibility for different items though a union type would be the way to go.

andrewMacmurray commented 7 years ago

Looking good, just those few changes and will merge in 👍

andrewMacmurray commented 7 years ago

@astroash @m4v15 I'm gonna close this for now, re-open a new one when you're ready for the work to be merged into master (we've kind of lost the separation between day-1 now so there's not much point in this current PR)