-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Blur sensitive search results #2640
Conversation
Size Change: +1.88 kB (0%) Total Size: 831 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! The changes work really work, both visually and with the screen reader.
Two points:
- blur seems to not be enough.
- should we also blur the audio genre?
:alt="image.title" | ||
:class="[ | ||
isSquare ? 'h-full' : 'margin-auto', | ||
{ 'scale-105 blur': shouldBlur }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use either blur-xl
or blur-lg
. Currently, some images are too clear and don't really hide the content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked a lower level of blur to help the user get a general idea of the shape of the contents of the image because with more blur, only some colour clues are left.
But I agree with your reasoning and will increase the amount.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a possibility of the genre containing anything sensitive? I don't think that is possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anything, genre will help people make a determination about whether or not they want to look at an audio file. I recommend we leave it unblurred
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once we use the blur values from the designs. Those were chosen carefully as part of the design approval process. According to Figma, there are two values in use:
filter: blur(60px);
for imagesfilter: blur(4px);
for text in the audio components (author, title)
I messed up, I forgot to look up the blur values in Figma. Sorry for the double reviews, it should be fixed now. |
Full-stack documentation: https://docs.openverse.org/_preview/2640 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice abstraction for blur styles!
Fixes
Fixes #2369 by @dhruvkb
Description
This PR adds logic to the image and audio result components to blur their content if their media item has been flagged as sensitive. It also replaces title and creator strings from the DOM with placeholders to prevent any sensitive information from leaking through assistive technology that bypasses CSS and interacts with the DOM.
Testing Instructions
fake_sensitive
toggle in the/preferences
page.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin