RobCoIndustries / pipboy

:beginner: Experimental pipboy Desktop app for Fallout 4
BSD 3-Clause "New" or "Revised" License
61 stars 7 forks source link

Added Text color to About. #26

Closed luckydonald closed 8 years ago

luckydonald commented 8 years ago

This is much code for nothing.

rgbkrk commented 8 years ago

Yup, we need to pass the color down from the parent React components in the future.

/cc @nelix

kitten commented 8 years ago

I think we should add a database provider that can be used like this:

<DatabaseProvider>
  db => {
    const color = db.getIn(['status'])
    return <div style={color: color}/>
  }
</DatabaseProvider>

Something like this should be easier to use

luckydonald commented 8 years ago

This is what @rgbkrk mentioned in #25, the second paragraph, right?

kitten commented 8 years ago

@luckydonald I'm not sure what you're referring to. I'm talking about a provider for the database, not the server selection.

Btw @rgbkrk We shouldn't pass the color prop down. That's just unnecessary bloat as we'll need to pass around a lot of stuff eventually.

rgbkrk commented 8 years ago

The main reason I said to pass the color prop is that pretty much all components are going to need it, scaling it to different gray values along the way. @nelix suggested to me that React components should provide properties like those top-down.

If you've got a simple approach that all components can use @philplckthun, I'm all for it.

To help me understand, what would the DatabaseProvider look like as part of the About page?

kitten commented 8 years ago

@rgbkrk Probably something like this:

<DatabaseProvider>
  db => {
    const colors = db.getIn(['Status', 'EffectColor'], []).map(x => Math.round(x * 255))
    return <div style={{
      color: colors.length === 3 ? `rgb(${colors[0]},${colors[1]},${colors[2]})` : ''
    }}>
  }
</DatabaseProvider>

Just a simple component that fetches the database for us instead of fetching it separately for each component. Passing those down is not that great considering that you have to keep track of each prop then.

Edit: And we should use Immutable for its great methods like getIn. Makes things easier

luckydonald commented 8 years ago

My question is, as all the parts of this app more or less need the pip boy data, shouldn't all data be inherited from somewhere in one place? Edit I see the multible-out-of-data-copies problem.

kitten commented 8 years ago

@luckydonald That's exactly what I'm proposing here: The data being inherited in one place instead of many. The data is already coming from a single source of truth. With fluorine by design this is the Dispatcher, which is our stream of events. So there's events for changes of the database on the Dispatcher. This is at the time reduced at multiple components, which is not an issue, if it weren't for the data structure that needs the reduced data to be converted into another format. The data needs to be "treeified". That's why it's possibly a good idea to treeify the data in a single component. I'm proposing this DatabaseProvider that would build the tree for us and distributes it.

This is more like a temporary solution. Once we're able to reduce the data changes into a tree structure inside our functional, pure reducer this won't be necessary anymore.