fix: Mark label as selected when programatically calling selectCategory()
#73
Conversation
Selecting the category visually and programmatically both call this method, so it's better that we do the visual style here like we do for deselection.
| .firstOrNull { it.isSelected } | ||
| ?.deselect() | ||
|
|
||
| // We compare names because the `items` field could technically be the same but the `Category` could be a different instance. |
There was a problem hiding this comment.
But can the same argument not be made for name as well? Shouldn't we just compare by reference?
| public fun <init> (Lgg/essential/vigilance/gui/SettingsGui;Lgg/essential/vigilance/data/Category;)V | ||
| public final fun deselect ()V | ||
| public final fun isSelected ()Z | ||
| public final fun markSelected ()V |
There was a problem hiding this comment.
This sounds like an implementation detail (granted, the entire CategoryLabel does, but it's too late for that one). When would it ever make sense for a third-party to call this method (rather than select)? If there's no such case, then it should be internal, not part of our public API.
|
|
||
| fun markSelected() { | ||
| isSelected = true | ||
| text.animate { | ||
| setColorAnimation(Animations.OUT_EXP, 0.5f, VigilancePalette.textActive.toConstraint()) | ||
| } | ||
| } |
There was a problem hiding this comment.
For symmetry I think we should also make a markDeselected (or have this method accept a selected: Boolean) which the current deselect calls, and then slap a @Deprecated on select and deselect because we shouldn't be using them any more, they only exist for backwards compatibility, we should always be using gui.selectCategory (which will in turn only use the mark* method(s)).
When we called
selectCategory()before this PR, the labels on the left-hand side of the screen did not get updated correctly. The de-selection was performed but the selection was not visible.Now, when you
selectCategory()the selection is visible, as well as the de-selection like before.Since
selectCategory()is public API, I feel like this is an important change to make :^)