search

Gustaf-C / anki-chinese-support-3

Anki add-on providing support for Chinese study
https://ankiweb.net/shared/info/1752008591
GNU General Public License v3.0
26 stars 7 forks source link

fix colorization of fields in editor #32

Closed kieranlblack closed 4 months ago

kieranlblack commented 7 months ago

The primary issue was that the way the tone css was being injected no longer worked for the current version of Anki. This commit modifies things to inject css in the proper manner.

image

Closes #3.

Gustaf-C commented 7 months ago

A small issue I found is that if I open the add notes window, and a note type without the tone colours css is opened by default, if I then switch to a note type with the tone colours css it won't show the colours in the editor window unless I close and reopen the window.

The number of times this would actually be an issue should be quite small, although it is a bit confusing.

kieranlblack commented 7 months ago

Hmm, would you mind sending a gif of the behaviour. I'm having difficulty reproducing the issue, this seems to be what you are describing:

There does actually seem to be some sort of issue with the font though.

demo

Gustaf-C commented 7 months ago

Recording

kieranlblack commented 7 months ago

I see, good catch. I have been able to reproduce the issue, I will look into what is causing it.

kieranlblack commented 7 months ago

The components are not being remounted so the onMount hook where we inject the styling is not being refired. I will continue looking tomorrow if I have time.

kieranlblack commented 7 months ago

I have made a little progress, the css should now definitely update and you won't see the bug you were seeing before. However, there is a race condition which has stumped me causing the userBase css to sometimes get wiped out resulting in most noticeably the font family and size not getting set correctly in the editor.

You can see it looks kind of strange here:

image
kieranlblack commented 7 months ago

Made a post about this on the forums.

https://forums.ankiweb.net/t/customstyles-strange-behaviour/37432

Gustaf-C commented 7 months ago

Good idea, hopefully some anki guru will come with some input. I tried looking at it for a while yesterday, but js isn't really my strong suit.

kieranlblack commented 7 months ago

It looks like nobody has weighed in on the post I made yet. I think I'm going to implement the fix I had in the post and then open a PR for Anki and see what dae thinks there.

Gustaf-C commented 7 months ago

Sounds good.

Otherwise I suggest we revert to 7faaf71 since that was at least an improvement, and then log the remaining bug in a new issue.

kieranlblack commented 4 months ago

@Gustaf-C just as a heads up, the fix for the issue with the custom style in Anki was merged in the other day so when the next Anki release is cut we should be able to merge this PR in 🥳

Gustaf-C commented 4 months ago

Great! ~I'll try to remember to merge this once the next Anki version is released~

Edit: I don't know why I would wait, I'll just merge it in now and wait with the release :)

kieranlblack commented 4 months ago

@Gustaf-C Ah, I guess I probably would've waited to avoid introducing known buggy code. We don't really know when the next Anki version will drop and now we are in a position where any bug fixes / features we want to push before it does will include this change unless we add a version check.

Gustaf-C commented 4 months ago

Yeah I know that it might become problematic in some cases, but it's the easiest way for me to make sure that the addon works well with the latest Anki version. If we want to push something else before the next Anki release I'll figure it out when we get there. If I regret doing it I'll just remember that the next time :)