AnKing-VIP / AnKing-Note-Types

https://www.ankingmed.com
GNU Affero General Public License v3.0
44 stars 19 forks source link

Explicit Image Blur #148

Closed shmuelsash closed 1 year ago

shmuelsash commented 1 year ago

Checklist

What Anki version are you using?

2.1.65 (aa9a734f)⁩

What Operating system are you using? (i.e. MacOS, Windows, Linux)

Windows 11

Problem case

We've all been there where we're in the library or some public place and BAM an explicit image shows up on your screen for everyone around you to see 😆

Solution

Add an image class called "explicit" and any image thats tagged with that in the html should automatically be blurred unless image is tapped/clicked

More information

Here's a conversation from the AnkiHub maintainers on this topic NSFW Thread.pdf

Here's how chatGPT suggests its implemented 😊 https://chat.openai.com/share/4c611c41-8f5d-40a1-addb-7ee495ed6853

AnKingMed commented 1 year ago

Sounds like a great idea. @abdnh can you implement this into all the AnKing note types? I think it would also be nice to have it implemented into the Wrapper Meta Add-on so that you highlight the image (or text) and then click the button to add the necessary html class

abdnh commented 1 year ago

can you implement this into all the AnKing note types?

Done. The blur class now can be added to images. Testing on AnkiMobile will be appreciated, because it's a bit fussy about click events.

I think it would also be nice to have it implemented into the Wrapper Meta Add-on so that you highlight the image (or text) and then click the button to add the necessary html class

Will look into that.

abdnh commented 1 year ago

BTW, let me know if the default blur radius/ratio is not good enough.

connwork commented 1 year ago

Hi,

Thank you for your great work!

I would recommend adding the appropriate CSS to the anki notetypes. This code seems to work well for me, not sure if you have something different: .blur { filter: blur(5px); transition: filter 0.3s ease-in-out; }

Also, I would recommend changing the event listener to "mousedown" instead of "click". I'm not sure how this will change things on mobile, but for desktop it makes it so it plays better with the click to zoom function (ie, image unblurs at the beginning of a click instead of on click release, so you can view it un-blurred on first zoom in). Code example below:

<!-- IMAGE BLUR -->
<script>
    for (const image of document.querySelectorAll(".blur")) {
        image.addEventListener("mousedown", () => {
            image.classList.toggle("blur");
        });
    }
</script>

Third recommendation that I do not fully understand: Do we need to apply the anking non-persisting event listener to this?

AnKingMed commented 1 year ago

@abdnh (this applies to https://github.com/AnKing-VIP/anki-wrapper/pull/11 as well) Do we even need to build a special class into these note types if we can just apply the style via the wrapper addon? Is the event listener necessary? I'd rather whatever we do the two play nice together. If it's easy to just apply via styling, lets just add it to the wrapper. If its better to do a class, lets do the class and have the wrapper add-on apply the class.

abdnh commented 1 year ago

Do we even need to build a special class into these note types if we can just apply the style via the wrapper addon? Is the event listener necessary?

The event listener is needed to blur/unblur the image on click. The wrapper add-on doesn't add the listener, so no way to unblur the image is provided by the add-on. Maybe it makes more sense to make the wrapper add-on add the event listener (will require modifying the add-on to also change the front/back templates, not just the styling section).

AnKingMed commented 1 year ago

I would probably just make it so the wrapper add-on adds the class and we can explain within the wrapper add-on that its designed to work with AnKing Note Types. Adding the event listener too sounds more complicated and we might run into issues with adding the event listener to notes that already have it?

The way you're doing it makes it so images are blurred by default correct?

AnKingMed commented 1 year ago

we could always do "visible:false" and leave out the keyboard shortcut on the wrapper addon so that only users that know they want it will turn it on and use the button. That may be the best option

abdnh commented 1 year ago

@connwork

This code seems to work well for me, not sure if you have something different:

This is the CSS added to the note types: https://github.com/AnKing-VIP/AnKing-Note-Types/commit/f52423caadff704b22a22af08efa8c86c9abd212#diff-efb952c64d7c5a342a9c09f32a383faeaa8afb5a44a764ea141a23c74fb6f76a

I would recommend changing the event listener to "mousedown" instead of "click".

I'm also not sure how if works on mobile. Will need more research.

Do we need to apply the anking non-persisting event listener to this?

I think no. ankingAddEventListener is for events attached to document.

AnKingMed commented 1 year ago

@connwork and @shmuelsash have you guys tested the suggested code on mobile too?

abdnh commented 1 year ago

The way you're doing it makes it so images are blurred by default correct?

Exactly.

we could always do "visible:false" and leave out the keyboard shortcut on the wrapper addon so that only users that know they want it will turn it on and use the button. That may be the best option

Sounds reasonable for a feature not a lot of people are likely to need.

AnKingMed commented 1 year ago

Let's do that then. I agree, it's probably not going to be a very popular feature

shmuelsash commented 1 year ago

I haven't been able to get it to work

I've been copying the raw data from Front, Back, & Styling into AnkingOverhaul notetype am I doing it correctly?

abdnh commented 1 year ago

@shmuelsash are you testing on AnkiDroid or AnkiMobile?

shmuelsash commented 1 year ago

First trying on desktop

connwork commented 1 year ago

@connwork and @shmuelsash have you guys tested the suggested code on mobile too?

Code below works on both desktop and ankidroid as expected.

<!-- IMAGE BLUR -->
<script>
    for (const image of document.querySelectorAll(".blur")) {
        image.addEventListener("mousedown", () => {
            image.classList.toggle("blur");
        });
    }
</script>
shmuelsash commented 1 year ago

I must be doing something wrong then

connwork commented 1 year ago

I must be doing something wrong then

Check your css to make sure the "blur" class is there. It wasn't merged to the main css until recently.

Also make sure to add the class="blur" to the card img tag.

shmuelsash commented 1 year ago

image image

connwork commented 1 year ago

Place only the styles portion in the styles section. The image blur script needs to go on your back template.

shmuelsash commented 1 year ago

Only back means it won't work on front of cards?

connwork commented 1 year ago

Yes. Not sure if there is a current use for front of cards?

shmuelsash commented 1 year ago

Why not? Some cards have pics on the front, no?

On Wed, Aug 30, 2023 at 12:16 PM Connor Workman @.***> wrote:

Yes. Not sure if there is a current use for front of cards?

— Reply to this email directly, view it on GitHub https://github.com/AnKing-VIP/AnKing-Note-Types/issues/148#issuecomment-1699472688, or unsubscribe https://github.com/notifications/unsubscribe-auth/AILOVMKAKF3AGIWC2T6HU63XX5RMLANCNFSM6AAAAAA3V5IPYY . You are receiving this because you were mentioned.Message ID: @.***>

AnKingMed commented 1 year ago

I think it should work front or back but I haven't tested it