ankitects
FEAT/ADD: colored answer buttons (with option to disable them)
This is based on #4370. I just took their code and added the preferences toggle.
The preference takes effect after restarting Anki, just like the other ones.
By default, the colors will be shown.
Images
With option off:
With option on:
Actually, for a new profile this bugs out. I'll investigate.
Edit: Fixed. I used true where I should have used false instead. It workes perfectly fine for me now.
Just a nit: as mentioned earlier, I suggest changing
.answerButton,
.answerButton:hover,
.answerButton:focus {
border-width: 2px;
}
to use 1px instead. I think this looks more refined and consistent with the rest of the interface, where a 2px border is only used when buttons are focused.
Will it be enabled by default? I am of the opinion that it would be most helpful if it was. The tinkerers amongst us will easily find the option to disable it.
Yeah, it doesn't make sense to have it disabled by default since it's supposed to help exactly the kind of person who is unlikely to tinker with settings or read the manual.
Regarding https://github.com/ankitects/anki/pull/4370#issuecomment-3348919522 (@Danika-Dakika):
With the last commit, I re-use the colors that ankis color palette is already using. I set the color for light and dark mode to the same color for now, though it can be easily changed later on if desired.
I chose not to use the colors that relearn and learn use. My reasoning is that those colors could change in the future. In that case, the color of the grading button borders should probably still be using green and red. Also: With the latest commit it should be easy to account for color blindness modes as well.
I'm afraid this will need to wait until we get the reviewer ported to Svelte so we avoid duplicate effort. @Luc-Mcgrady has already started to look into it in #4289, and some of this work might be able to be reused there.
I'm afraid this will need to wait until we get the reviewer ported to Svelte
I could agree if there was a clear deadline for when the migration to Svelte will be done AND the deadline was less than a year away. That is not the case. @dae I'm sure you're getting tired of me complaining, but I want to say: "let's not do any UI-related work for an indefinite amount of time" does not strike me as a reasonable stance.
Btw, Dae, I was hoping you would be more willing to merge this PR since it's not you who had to do this work. I was under the impression that you would be ok with colored button borders but just didn't want to work on that on your own.
It would be me doing the work again of implementing it in the svelte PR, hence duplicate work. If there are any problems with this implementation then any fixes would be temporary and a waste of time. The amount of time until the reviewer is done is not indefinite as I am actively working on the svelte reviewer.
Porting the reviewer to svelte is difficult as it is. I don't want extra work keeping up with changes that happen outside the pr :sweat_smile:
@Luc-Mcgrady Would you be open to this PR including the necessary code to replicate this UI change in Svelte as well?
If this is going to be implemented in the Svelte reviewer anyways, and the work for this one is already done how is it duplicate work?
@Luc-Mcgrady I can see why it would feel frustrating to put a lot of work to replicate/port an interface that is also a moving target. Separately from this PR, would you be open to (as part of the great work you are doing in #4289) add this UI change to your planned tasks as part the work in the reviewer port, to be worked on when you prefer?
and the work for this one is already done
Any changes / fixes for this will have to be replicated twice.
add this UI change to your planned tasks as part the work in the reviewer port, to be worked on when you prefer?
It would be better in a separate PR after the reviewer is ported.
Maybe there is a way to make both sides happy? What if we merge this PR and you, @Luc-Mcgrady, ignore this feature in your porting efforts? Once your port is finalized, I could open a new PR to reimplement this feature. I assume I should be capable of that (even though I'm a novice), as the most difficult one (the porting itself) would be done by people like you that know what they're doing.
Of course, my suggestion might be a bad idea. I cannot really tell due to limited knowledge. So if it's bad, feel free to openly call it out.
(even though I'm a novice)
Liar :wink:
There's a lot of code in this PR which can be re-used by my PR (notably the config bool). I checked and currently the 2 branches have no merge conflicts (yet). It's not only my/your time to waste though. People who are not us will have to review this PR and as I said previously keep up with any associated bugs. Give me time to complete the svelte migration and then you can do whatever you like :).
I could agree if there was a clear deadline for when the migration to Svelte will be done AND the deadline was less than a year away. That is not the case.
It's dragged on much longer than I would have liked. A significant factor there is because I've been prioritizing correspondence and PRs over development work in the limited time I've had available. Can you think how your own contributions, however well intentioned (and in most cases useful/appreciated), might have also played a small part here? But the blame ultimately lies on me as the project manager, and I think I need to to shake things up so that I can do more development and spend less time on the other stuff, which I don't really enjoy. That includes getting better at saying no.
Liar 😉
It is true, but thank you for the complement!
It's not only my/your time to waste though. People who are not us will have to review this PR and as I said previously keep up with any associated bugs.
That makes a lot of sense. I'll gradly wait for your PR.