Better solution for page and button events binding

[yundifu]

Hi Guys, please check this branch for the current spike for binding for different pages and buttons. https://github.com/twlevelup/watch_edition_react/tree/ugly_button_state_spike

Just wondering is there a better solution here than this one? please share your ideas and suggestions on this :)

[alex-mitchem]

At this stage Aaron, Sinan and I have all spiked out a different solution to this problem and decided to use not of any of them and go with redux instead.

I think the basic idea at this stage (or my interpretation of it at least) is to have the buttons be part of the redux state of the framework, using a generic container component to wire them with the buttons. All we'd need to do then is expose some way of assigning the buttons actions in the root component.

On top of being cleaner, it should also be a lot easier for the students to learn and use. I think it also simplifies any issues around closures as well, although I'm not sure I properly know how closures work when using redux. As long as the redux is confined to the framework it shouldn't make any difference to the students.

[aaron-m-edwards]

Do we have a spiked solution for redux? I still don't see how that solves the event binding thing

[alex-mitchem]

To my knowledge there is no redux spike. Can you elaborate on the event binding thing?

[sinan-aumarah]

I really like Aron's solution. I think from a student perspective, it's easy and simple. Even if we go with a Redux spike, we will abstract it and only expose something similar to what Aron did.

[alex-mitchem]

I disagree that it will be simple for students. Aaron's way works well for global functionality like navigation, but forces you to manipulate the component's state to achieve any functionality. We're dealing with university students, manipulating a model's state won't be very intuitive. Universities tend to teach a more hypothetical version of MVC.

That said, I'm starting to think Aaron's way may be the way to go (although I think the action maps for each page should be defined elsewhere and imported), we just need to be aware that we're going to have to coach the students on how to do it properly, otherwise we risk it turning into a giant mess.

As for using Redux, I just think it would be a bit cleaner and easier for anyone who develops after us. It wouldn't be much of a change, basically turn createWatchPage into a container factory that accepts a component class, its route path and its button map as arguments and spits out a HOC class. Plus Redux is awesome.

[aaron-m-edwards]

I think the way I have it in the spike branch is much simpler than what is in master.

What do you mean by forces you to manipulate the component's state to achieve any functionality ? Because both solutions fall into this same trap (Check out the CounterScreen I whipped up last night). However mine does abstract this away from the sudents a bit

eg bottom: changeScreenState(state => ({ ...state, number: state.number - 1 })),

This contrasting with the version in master that will force students to do things like

buttonActions = { BOTTOM: () => this.setState({ number: this.state.number - 1 }) }

I do want to point out I don't have a problem with the this.setState etc, the main problem I have with the version in master is the whole handleMapper which is a very unintuitive pattern that I can see will cause a lot of issues for students (mainly in forgetting to rebind) as well as in maintenance in general.

But the main issue I have with one in my spike branch is that actions are disjoined from the WatchScreens. It is much easier to test the actions in the one in master.

I have been playing with a bit of a Hybrid solution last night, using refs rather than the mapper. It does however require all Screen's to be Smart components rather than pure functions and depends on certain methods being on the component instance. But is more 'Object Orientated' rather than functional react (which is probably a little more familiar to students). It also removes the need of React.clone which is also another gripe I had with the current one

I also want to but a really big -1 on redux. I don't think it solves any of the problems we are currently having

[sinan-aumarah]

I think Aron's solution is perfect for now. I'm reading about Redux and hopefully will spike by the end of the week if I get time.

Aron please don't use refs. Reactjs documentation discourages it strongly. Also now that I think about it, if we want something simple and familiar to students why don't we go for something like MobX? I think most students have done Java subjects and MobX would be easier to read for them? not sure really

[sinan-aumarah]

I think I finally cracked it using Redux and wrapped it so that students don't have to worry about it.

Have a look @ https://github.com/twlevelup/watch_edition_react/pull/24

[sinan-aumarah]

can we close this now?