`switch_focus` iterates from the last focusable line to `_lineCount` instead of the first line about liquidmenu HOT 6 CLOSED

I also noticed this issue. Can you make a pull request of your fix @DarioMaechler?

from liquidmenu.

Can you make a pull request of your fix @DarioMaechler?

I turned it into pull request #39 It can be installed into platformio until this is released (or fixed differently) using:

lib_deps =
  LiquidMenu=https://github.com/thijstriemstra/LiquidMenu/archive/patch-1.zip

Can you review the pull request @VaSe7u?

from liquidmenu.

Hello, @DarioMaechler and @thijstriemstra.

This is intended by design for the purely aesthetic purpose of not showing the focus indicator when it's not used.
However you are not the first people to find this behavior confusing, so I might add an option to enable/disable it.

Thank's for the input.

from liquidmenu.

Thanks for the feedback.

This is intended by design for the purely aesthetic purpose

I see. I think the default should not be the aesthetic version since it's confusing IMO, I thought my code/board was buggy. An option sounds good though.

from liquidmenu.

Thanks to both of you as well.

I agree with @thijstriemstra. Based on the method names and the comments in the declaration I find the behavior confusing as well. Small example illustrating the problem:

1. execute switch_focus(true) until get_focusedLine() == _lineCount. Works fine so far (the focus just isn't visible).

2. execute set_focusedLine(get_focusedLine()) --> this will return false and not work

In my logic the getter returns an illegal value if the setter for the same item won't accept that same value.

from liquidmenu.

Hello and sorry for the delay.

I thought about it and I agree that hiding the indicator while iterating the focusable lines really is confusing and this is not an intuitive place to put that functionality in.

I decided to make this side effect compile time configurable though a define inside src/LiquidMenu_config.h:
https://github.com/VaSe7u/LiquidMenu/blob/5978667585b9ea3cbdfcff5316bcb5b89fd10215/src/LiquidMenu_config.h#L83
It defaults to enabled in order to keep backwards compatibility.

Thank you for the input and the help with the code modification needed.

from liquidmenu.