Fix occlusion drift again #3443
Conversation
|
Ouch, that's no good. If it's reproducible, would you mind bisecting to figure out the commit that caused it, and providing steps to reproduce? |
|
Latest 2 commits fix #3449, upon testing I don't see any new regressions/bugs as a result of my changes. The entire architecture was a bit of a mess, so I refactored it a bit to be more streamlined. It seems like the issue was caused from I have also found a few more unrelated bugs that I will report. Once I knock out these next couple bugs I'll see if I can identify when #3449 was introduced. |
|
Fixed #3451, #3452, and #3456. Also, I made it so on creation of text labels it enters edit mode on the label and switches the tool back to the cursor. I found having to click onto the label to edit it after creation tedious and then clicking off the label to deselect I would always unintentionally create additional labels. I've done some more testing and now everything seems to work as intended. |
There was a problem hiding this comment.
Thanks for looking into these issues Taylor!
@krmanik I know you're busy recently, but I wanted to give you a brief chance to review this PR, as it's mainly adjusting code I think you wrote in the past. If I don't hear from you in 48 hours, I'll merge this in, assuming there are no other pending changes.
ts/routes/image-occlusion/store.ts
Outdated
| */ | ||
| export function saveCompleted() { | ||
| saveNeededStore.set(false); | ||
| } |
There was a problem hiding this comment.
I find these names a bit unclear, and moving them into functions seems to obscure what's going on a bit. How about either
a) accessing the store directly, eg
emitChangeSignal() -> saveNeededStore.set(true)
or b) call it something like
emitChangeSignal() -> setUnsavedChangesPending(true)
There was a problem hiding this comment.
I've removed the helper functions and have instead accessed the store directly for all calls.
Fixes #3370
Addressed bugs preventing occlusions from being added as a result of my previous fix.
I'm opening this as a draft for now, because I'm getting some strange behavior with the occlusions appearing on images they aren't supposed to. I'm able to recreate this on the main branch as well as the latest official version of Anki, so it doesn't appear to be my changes causing it. I will follow up once I have more information.