mobxjs / mobx

Simple, scalable state management.
http://mobx.js.org
MIT License
27.53k stars 1.77k forks source link

How can rollover button functions alter button colors on hover and focus without changing state during the computation of a reactive view? #30

Closed dschalk closed 9 years ago

dschalk commented 9 years ago

What would be the recommended way to change button colors on hover and focus? The console log says I am changing state. How can button colors be reactive and not be part of state?

My rollowver buttons are part of my websockets-react and my mobservable-react-buttons apps at https://github.com/dschalk/websockets-react and .https://github.com/dschalk/mobservable-react-buttons respectively. The apps ore running at http://machinegun.ninja and http://schalk.ninja. I trimmed all but the essential code and posted the buttons code, along with the error warning, at https://github.com/dschalk/bull.git

mweststrate commented 9 years ago

Hi David,

see: https://github.com/dschalk/bull/pull/1

dschalk commented 9 years ago

I was going to say "good morning'", but I think it is afternoon in the Kingdom of the Neatherlands. It is 7:35 a.m. here in Bloomington, Indiana.

I don't like bothering you with my dilemma. I can see from the other issues that you are busy building a truly wonderful library. I am glad to see (eg. Issue 5) that some other smart people have taken an interest in what you are doing. You not only have the smarts, but also the creative talent to see past accepted 'best practices', such as MVC, separation of concerns, 'mutable data and global variables are always bad', and other ideas that hold people back. I enjoy the peaceful certainty of coding in Haskell, but Javascript's flexibility sometimes makes me smile. I stop the timer in the Game of Score (websockets-react) by setting the timer variable 'DS_T" equal to a text announcement about why the round of play has ended. It replaces the countdown integers which showed that a player was working on a solution. Not very type safe, but a nice way to accomplish two things with one deft maneuver. You can't do that with Haskell.

Anyway, I hope I am not troubling you too much. Dynamic buttons are essential for good user interfaces, and they ought to be done right. I don't want to concoct some half-assed hack that is inconsistent with your vision of how data should flow through an application.

I tried to pinpoint the issue with my little repo at https://github.com/dschalk/bull.git.

mweststrate commented 9 years ago

Hi David,

Thanks ;-). Did you see my pull request on your repo? I didn't test it, but I think it contains the solution for it :)

dschalk commented 9 years ago

I tested your pull request code and the console log message about changing state during the computation of a reactive view no longer appears. Thanks for showing me how to fix that.

I still get a warning in the console log saying "mobservable.observeUntilInvalid is deprecated and will be removed in 0.7." Am I going to have a problem with version 0.7?.

mweststrate commented 9 years ago

Upgrading to the latest mobservable-react should fix that warning.

dschalk commented 9 years ago

With version 0.1.8 of mobservable-react the little buttons demo flopped, giving the console log massage, "Uncaught TypeError: e.observe is not a function".

I pushed the non-functioning code to the "bull" repo. The npm dependencies are:

"babel": "^5.8.23", "babel-core": "^5.8.23", "babel-loader": "^5.3.2", "file": "^0.2.2", "mobservable": "^0.6.7", "mobservable-react": "^0.1.8", "webpack": "^1.12.1", "react": "^0.13.3", "react-mixin": "^1.7.0", "webpack-dev-server": "^1.11.0"

mweststrate commented 9 years ago

hmm I would expect npm to throw a PEER_INVALID on this one. Can you upgrade mobservable to 0.6.9?

dschalk commented 9 years ago

I don't know what happened. I just now cloned the current version of bull, ran npm install, webpack, and warp, and saw at localhost:3000 that it "works" without giving any warning messages. It uses mobservable version 0.6.8 and mobservable-react version 0.1.8.

The buttons stopped working properly when groupWatch was removed from 'data'. I said I tested the pull request, but all I did was observe that the warnings about changing state during a computation of a reactive view were gone. I didn't make sure the buttons still worked. I forgot that groupWatch was reactive for a reason. When it stopped being reactive, the buttons stopped functioning properly.

Problem 1: 'solo' is still the default group, but the solo button is not lit by default. Problem 2: Typing in 'solo', GroupA, GroupB, or GroupC no longer causes the corresponding button to light up. Problem 3: When a new button is clicked, the previous button stays lit. Entering and leaving the previous button shuts it off. It is possible to have all four buttons lit at the same time. Problem 4: Typing in group name 'fred', or anything other than solo, GroupA, GroupB, or GroupC, no longer causes all of the buttons to dim. Typing in names has no effect on the buttons, neither causing them to be selected nor unselecting them. All communication between the text box and the buttons is lost.

Everything worked perfectly when groupWatch was reactive. I'll experiment until I find a solution that makes the buttons work properly without console log warnings about changing state during a computation of a reactive view, or until I give up. Having groupWatch be reactive was the magic that made the buttons work properly, and do so much more than any css or javascript hover and focus sensitive buttons I have used in the past. When groupWatch was reactive, the buttons were lit or dim depending on the name of the currently selected group. I'll look for another way of accomplishing this.

mweststrate commented 9 years ago

Hi David,

I just looked very briefly on your actual source code of the bull project (beyond the error I fixed). I was a bit mislead by the naming initially. you are right, groupWatch should be reactive, what it does is ultimately just a derivation of the real state of your app. What isn't right is that it tries to alter state that stores colors and such. Instead, it should return a description of the colors and such.

So what it should look like is a bit like this:

var data = makeReactive({
  groupWatch : function() {
    if (this.group === 'GroupA' && this.test) {
      return {
           Abackground : 'green',
           Aborder : 'lawngreen'
           //etc etc etc
      }
   else if (.....) return { .... }

These derivations are a bit more complex than that, so probably you need some state like 'currentlyHoveredButton' or 'isMouseOnButtonXYZ', so that you can base your derivations (views) on those facts as well.

Then in the end, based on how you structure it, your style definition will use your derivation, so you get something like: style={this.style8(data.groupWatch.Abackground ...

I hope I pointed you in the right direction, as I had limited time to write this explanation. But it boils down to separating state/facts from views/derivations.

dschalk commented 9 years ago

The buttons function perfectly in the latest pushed version of bull. Instead of making groupWatch reactive, code in eleven places calls the function.

I must be missing something here. If it really is becoming impossible to have a set of buttons automatically change colors on hover and focus in unison with each other and a text box, then there must be other useful functionality that we will no longer be possible automatically. Having things happen automatically without the need to explicitly call functions doesn't seem like anything you would ever intentionally sacrifice in order to achieve a goal, no matter how worthwhile the goal may be. So, as I said, I must be missing something. There must be a way to eliminate the eleven lines of code in which groupWatch is called. Maybe, by the time you read this, I'll know what it is.

dschalk commented 9 years ago

I hadn't seen your latest entry when I submitted mine. Thanks for pointing me in the right direction.

dschalk commented 9 years ago

Well, all's well that ends well. The buttons at https://github.com/dschalk/bull are fully functional. The code not only complies with Mobservable best practices (at least there are no longer any console warnings), it is much streamlined. It went from 735 to 164 lines of code without losing any functionality.

Thanks for taking the time to point me in the right direction. I followed your advice, and it turned out that the buttons and text box could all be made to work in unison with only a few variables.

dschalk commented 9 years ago

Oops! I didn't compact the code quite that much. I forgot about the Fibonacci computations and commentary in "mobservable-react-buttons". Still, it is drastically shortened.

mweststrate commented 9 years ago

Awesome! I was actually wondering; personally I believe that pure views will result in the most maintainable code. But should I force users to work this way by explicitly forbidding to change state? Or would that be too harsh and make it hard to adopt mobservable? Technically mobservable doesn't need such a constraint...

On Tue, Sep 22, 2015 at 4:08 PM, David Schalk notifications@github.com wrote:

Oops! I didn't compact the code quite that much. I forgot about the Fibonacci computations and commentary in "mobservable-react-buttons". Still, it is drastically shortened.

— Reply to this email directly or view it on GitHub https://github.com/mweststrate/mobservable/issues/30#issuecomment-142299576 .

dschalk commented 9 years ago

Maybe you could make pure views the default and have a simple opt out option, something like "use strict" in Javascript. Development team leaders everywhere could enforce best practices by requiring a special dispensation, or maybe a majority team vote, if someone had a special need to use the opt-out directive. I love the chapter from the Tao Te Ching which, loosely translated, says that a bad emperor is feared by the people; a better emperor is loved and admired by the people; but the best emperor is unknown to the people. When the goal is accomplished, the people say, "We did this ourselves." Meetings are generally a waste of time, but a few meetings to devise strategies for making code maintainable, in which a true consensus is reached, could result in a team which says, "We thought of this ourselves", largely oblivious to the leader's deft, unobtrusive manipulation of the process.

For your published library, my preference would be to provide the means in the API to manipulate state during the computation of a reactive view. I stopped using the Yesod Haskell web application framework because it was so 'opinionated'. Everyone is entitled to an opinion, but Michael Snoyman apparently can't bear to know that people are writing sloppy, buggy code with his library, so he imposes his opinions on library users by declining to expose functionality they might want. He explains this well, http://www.yesodweb.com/, and his opinions are first-rate and deserving of the utmost respect, but In order to get things done in 'off the beaten path' use cases, I sometimes needed to alter his library code. I would have preferred to have been given more options in the API.

The Glasgow Haskell Compiler tolerates Haskell coders' use of functions that compiler authors rely on to communicate directly with machines and with the C language. These are not Haskell pure functions, and could easily result in buggy, hard-to-maintain code. The two most-often-used functions in this category are 'unsafeCoerce' and 'unsafePerformIO' and its variations. I generate random numbers for the dice roll in the 'Game of Score' app. No referential transparency there. It uses the notorious 'unsafePerformIO hack'. I don't worry about it because I don't extract the numbers from the IO Monad which contains the numbers until I get to the final function 'main' and broadcast them to 'Score' game players. In one of the first versions, I used 'unsafePerformIO' to extract the numbers right in the middle of the program. The numbers were never used by the program and the compiler never saw them again after they were generated, so all I had to worry about was someone reading the code on Github and forming a low opinion of my coding ability. The point I want to make here is that Haskell programmers and people reviewing their code are reminded of the riskiness of using these functions by their names. Maybe your directive for overriding best practices could be "use questionable coding practices' or 'use dangerous'.

I cleaned up part of my'mobservable-monads' repo last night and added some commentary in the 'Working With Numbers" section. I know you are extremely busy, and I don't want to impose upon your time, but here's a suggestion: I think you would enjoy the "Working With Numbers" section of "mobservable-monads, and possibly find the Haskell monad concept of pulling a value out of one monad and mapping it to an instance of possibly even another type of monad monad either (1) thought provoking, (2) essentially what you are already doing, (3) boring, (4) something else. Commentary for the whole "Working With Numbers" section is only 12 or so sentences. I reproduced the section in README.md at https://github.com/dschalk/mobservable-monads, so you can see what I am talking about at that repo.

And here is a heartfelt recommendation : Even if you are certain that you will never have any use for Haskell, I think you will find (if you haven't already done so) Part II of Simon Marlow's "Parallel and Concurrent Programming in Haskell" to be almost spine-tingling in the poetry of its clear exposition of some of the most simple yet powerful code you will ever see. This is elegance par excellence. The concurrency section, Part II, starts on Chapter 7, "Basic Concurrency: Threads and MVars". In my "Game of Score" project, I use a modified MVar called a 'TMVar, which is explained in Chapter 10, "Software Transactional Memory". It facilitates atomic updates.

The TMVar in "Game of Score" is named ServerState. It contains the current list of connected players, the groups they are in, and their current scores. My app has immutable state, according to the Haskell concept of 'immutable', but players, their groups, and their scores constantly change when players compete; and current state is always available upon request from the ServerState TMVar. The trick is that state is stored in the immutable TMVar. Updates involve removing the state, making a new state that differs from the removed one in the desired way(s), and inserting the new state in the empty TMVar. In a Javascript application, mutating the encapsulated state without removing it would likely be the way to go. I'm still mulling this over and sleeping on it, but it occurs to me that mobservable might benefit from doing something similar; something which would allow more developer flexibility regarding dynamic aspects of state without using view functions to mutate state (in the haskell sense of mutating) during computations. In a Node.js websockets application, such a thing might even be useful in the server for managing state, much as Haskell TMVars handle state in my game app.

I think it is best to err on the side of giving developers too many options in the API, rather than too few. Warnings and flags with alarming names are good, but in the end, give them an API with functionality that allows them to do whatever wild and crazy (but possibly beneficially innovative) things they want.

dschalk commented 9 years ago

I wanted to participate in an ongoing discussion, so I put the above comment in "How to constrain independent properties?".

dschalk commented 9 years ago

I made the mobservable-monad's README.md more legible.

If I have any further comments on this topic, I'll make them in the "How to constrain independent properties" issue.

dschalk commented 9 years ago

I learned a lot from this dialogue, but I think I should point out that I could have solved my original problem by not making the button-color object, which was unfortunately named "mouseHandler", observable. Had it been a simple object, there would have been no mutating-state issue and everything would have worked perfectly. The button colors reacted to mouse hovers and clicks because of a method in the observable object named "data", and not because "mouseHandler" was reactive.

By failing to see this simple solution, I learned a better way to use mobservable to make rollover buttons. The better I come to understand mobservable, the more I appreciate it.