Closed SaurabhTotey closed 6 years ago
The issues that we have had with this sim design is that is not a good idea to have information inside the graph because in can get lost with the visible spectrum or with the zooms. My proposal is to move this temperature values outside the graph.
@arouinfar help me to think in this solution to have the last saved curve in solid line and the one before using dash lines. When the user save a new curve when it's already two in the sim, then the 9109 K curve (the one used in the picture as example) now is going to be dash, the 8592 is going to disappear and the new saved curve is going to be solid.
waiting for @ariel-phet opinion
@DianaTavares @arouinfar what about the following design.
One thing that has bugged me for awhile is that the temperature readout being right on the slider thumb of the thermometer leads to a very big "dead space", in addition, you are likely to cover that readout with your finger if using a touch device. Also, the controls for the graph do not really have good proximity to the graph. In this design I try to solve some of those issues, and use alignment to cleanly associate the thermometer with the graph.
@arouinfar @DianaTavares obviously the above is not polished (needs to be better aligned and such), but see if you like that general direction.
Taking the @ariel-phet suggestion and working with @arouinfar, this is the proposal:
If three curves are in the graph, I think that is important that all of them have the temperature value, for that reason the red curve was added.
With the word "intensity" and the value now closer, the suggestion is change the checkboxes order and add the value of the intensity close to the word. When this checkbox is not selected, the rectangle around this options is going to be smaller, but it is not going to move the curve values.
The values of the the temperature of the curves will appear only when at last one curve is saved, like is shown in the next mockups:
When any curve is save:
When at last one curve is save:
what do you think @ariel-phet?
@DianaTavares I like this direction, we will probably still need to do some polishing once we play with it, but overall I think the layout is much better.
@SaurabhTotey it looks like you have been working on this layout, I would say continue. When you have a working version, please publish a dev version for us to look at.
Hey @ariel-phet, I believe I have finished the basics of the design outlined above. It doesn't look exactly like the mockup, but it has the same general design and features, so I published a dev version.
Here is a link to the dev version for how this looks so far.
@SaurabhTotey nice work. Certainly we still need some polishing, but playing with the dev version I like this design direction and think it is a much better layout.
I love it @SaurabhTotey excellent work, thanks!!
The new layout is looking really nice @SaurabhTotey!
This implementation is already done an looks amazing! Nice work @SaurabhTotey! closing
According to the design document, each saved graph should have a temperature label associated with it. However, I am not quite clear on where the temperature label should go. For testing purposes, I just used the old intensity label placement to place the temperature labels. However, this approach may not be sufficient because I am noticing a few issues. For example, in the image below, when saving two curves, the temperature labels are adjacent and it is difficult to tell which label corresponds to which curve. One thing I tried was to move the label text closer to the peak of the graph, but then new issues arise. After moving the label closer to the peak, the label often intersects with the graph, making it hard to read, as shown in the image below. The design document has markups that show the label to the right of the curve, but I wasn't able to figure out if there was a general pattern or specific way that labels should go in relation to the curve. In short, I am not quite sure where to place the saved curve temperature label in relation to the actual saved curve.