Implement refreshAt for ActiveLayerKeys

[EvyBongers]

No description provided.

[obra]

I feel like the not-resetting would lead to surprise and confusion for users? It *seems* like it's worth doing.

…

On Thu, Nov 2, 2023 at 3:12 AM Evy Bongers ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In plugins/Kaleidoscope-LED-ActiveLayerKeys/src/kaleidoscope/plugin/LED-ActiveLayerKeys.cpp <#1356 (comment)> : > + if ((k == layer_key) && (k != Key_NoKey) && (k != Key_Transparent)) { + ::LEDControl.setCrgbAt(KeyAddr(key_addr), active_color_); + } else { + ::LEDControl.setCrgbAt(KeyAddr(key_addr), {0, 0, 0}); ::LEDControl.setCrgbAt(KeyAddr(key_addr), {0, 0, 0}); is needed to reset the color of keys not in the current layer. Some time ago, I mentioned on discord that I'm thinking about implementing an option to support painting keys for all layers in the stack. This can almost be achieved by adding a condition on the else clause like this ⬇️ Suggested change - if ((k == layer_key) && (k != Key_NoKey) && (k != Key_Transparent)) { - ::LEDControl.setCrgbAt(KeyAddr(key_addr), active_color_); - } else { - ::LEDControl.setCrgbAt(KeyAddr(key_addr), {0, 0, 0}); + if ((k == layer_key) && (k != Key_NoKey) && (k != Key_Transparent)) { + ::LEDControl.setCrgbAt(KeyAddr(key_addr), active_color_); + } else if (!remember_past_colors) { + ::LEDControl.setCrgbAt(KeyAddr(key_addr), {0, 0, 0}); The downside of this is that unshifting a layer won't cause the leds to be reset, except for keys in the now-topmost layer. If you're interested, I'm happy to add a commit to implement that, even if it isn't how I envision it. — Reply to this email directly, view it on GitHub <#1356 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAALC2CWF44JIVWE7RFR2BLYCNW2TAVCNFSM6AAAAAA62TDWBOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBZHA4DMOJQGY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[EvyBongers]

I feel like the not-resetting would lead to surprise and confusion for
users? It seems like it's worth doing.

I'm going to leave it out and look for a better way to achieve what I want.

[EvyBongers]

That was easier than I expected. I had completely overlooked Layer.lookupActiveLayer 😄