-
Notifications
You must be signed in to change notification settings - Fork 35
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
💄 Make graphic media less visible #194
Conversation
return ( | ||
<div className="my-2"> | ||
<LabelChip className="ml-0 mb-2">{label}</LabelChip> | ||
{(['blur', 'grayscale', 'translucent'] as GraphicMediaFilter[]).map( |
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.
This list is used a few places and cast into GraphicMediaFilter[]
. Would it make sense to export it somewhere as a const with the proper type?
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.
totally, was just being lazy :(
labelsRequiringMediaFilter.forEach((label) => { | ||
;(['blur', 'grayscale', 'translucent'] as GraphicMediaFilter[]).forEach( | ||
(filter) => { | ||
initialValue.graphicMediaPrefs[ | ||
buildGraphicPreferenceKeyForLabel(label, filter) | ||
] = true | ||
}, | ||
) | ||
}) |
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 find this a bit difficult to read! Would be sweet if we could get it to be something like:
labelsRequiringMediaFilter.forEach((label) => {
GRAPHIC_MEDIA_FILTERS.forEach((filter) => {
const key = buildGraphicPreferenceKeyForLabel(label, filter)
initialValue.graphicMediaPrefs[key] = true
})
})
}) | ||
|
||
const [localPreferences, setLocalPreferences] = useLocalStorage( | ||
'ozoneLocalPreferences', |
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.
Would there be any value in clearing these on logout, or keeping separate keys per user?
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 I've avoided that until now since I see these configs as individual/device level preference, not account level. if the same user is accessing ozone using different accounts, I feel like they would want these configs across accounts as long as it's the same device. but yeah, I don't think the other approach to tie it to user level makes sense in some contexts.
This PR adds a localstorage based preference that allows users to choose 3 possible filters on media content with certain labels. Previously, we were only applying a blur filter for all these labels and now, along with blur, media can be black and white and translucent, according to user's preference.
By Default we will apply all 3 filters for those labels
Additionally, this PR adds modal image viewer for profile avatar and profile banner image.