CCP-NC / magresview-2

MagresView 2.0 - NMR crystallography visualisation app
https://ccp-nc.github.io/magresview-2/
MIT License
0 stars 2 forks source link

Issues with MVRange #3

Open dch0ph opened 2 years ago

dch0ph commented 2 years ago
  1. There is a rounding issue associated with: v = Math.round(v/step)*step;

This is mostly OK, generating outputs like 0.05, 0.25 for a step of 0.05, but will occasionally spit out things like 3.5000000001, which looks terrible. The easiest work around (if JS has suitability functionality on toString) is to pass a number of decimal places to round too.

  1. I disagree with the concept of defaulting to the minimum value: const in_val = (props.value != null? toNumber(props.value) : min);

This is arbitrary. The user should be forced to choose a default / initial value. For example, with J and dipolar coupling, a sensible default is 1.5 A, even if the minimum is 1 A. A default of 1 A will generally select no atoms!

  1. Linked to the above, it would probably be useful to introduce the concept of "reset" to controls, so a user can always reset a given panel to sensible defaults / initial state.

  2. I don't like the logic of in acceptValue of rounding even user input values. Why should say 1.32 A get rounded to 1.3 A? For floating point inputs, it might be better to just to work with a "number of decimal places" and drop the "step".