search

oliexdev / openScale

Open-source weight and body metrics tracker, with support for Bluetooth scales
GNU General Public License v3.0
1.68k stars 292 forks source link

Settings for overview Y-axes visibility #971

Closed undefiened closed 1 year ago

undefiened commented 1 year ago

Add a new setting that allows enabling or disabling Y-axes on the overview screen. It allows selecting left, right, both, or no axes on the overview screen. When any of the two axes are enabled, it also automatically enables grid lines (I think it makes sense to do so).

2023-07-22_13-52

oliexdev commented 1 year ago

Thanks for your PR. I think the y-axis option in the settings are not the right place because this option is specific for the overview graph and not for all graphs. I would prefer to have an option in the already existing popup menu (bottom right corner of the graph) of the overview graph. It could be simplify to only show/hide the y-axis. I don't think to have two more options with only left and only right is necessary. What do you think?

undefiened commented 1 year ago

@oliexdev Yeah, I think it makes sense. The reason why I wanted to have control of the left and right axes separately is that the right one is often not needed (I only really care about the weight, so the right axis just eats up space). But this probably can be solved separately. I can imagine two possible solutions:

  1. add a single selector like the one I had before which would work for both overview and full graph views
  2. automatically disable right y-axis if there is only one measurement

Would any of them make sense to you?

Otherwise, I think I changed things according to your comments. I am not sure how to call the button though. Is "Y-axis" fine?

oliexdev commented 1 year ago

thanks @undefiened for the quick redesign. The second option would be better because there is no user interaction necessary. Yeah the label is fine. I renamed just the internal label name to a more general name (removed the word "overview")

undefiened commented 1 year ago

Thanks for the quick reviews!

I added autohiding of the right y-axis in the overview. I decided to not make measurementViews in ChartMeasurementView public, so I added a method that returns the number of measurement views activated in overview. Is that fine? Or should I just make it public?

Should I also add autohiding in the full graph view?

Also, I made a couple of small fixes compared to the previous commit. Please let me know if you don't like something.

undefiened commented 1 year ago

Sorry, I just discovered that there is also some logic regarding that in isOnRightAxis method in MeasurementViewSettings. How should I deal with it? Should I hide left axis instead when the only measurement view is one which should be on the right axis?

oliexdev commented 1 year ago

True, well I put the weight on the right axis and the rest measurements on the left axis by default. The best would be to count how many are on the left side and how many are on the right side and depending of the counting enable/disable the left/right axis but I don't know if that's easy to implement. If not than remove the "auto hide one axis" feature. ;)

undefiened commented 1 year ago

I implemented autohiding of the appropriate axis, it is not difficult, but I am not sure if it looks better. I feel like having y axis at least on the left of the chart is more natural. I would rather remove the "auto hide one axis" feature then unless you like the current behavior.

oliexdev commented 1 year ago

I agree with you. Thanks for your effort :1st_place_medal: