-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add content warnings for videos/links #457
Conversation
"has_content_warning", | ||
sa.Boolean(), | ||
nullable=False, | ||
server_default="false", |
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 migration does not add content warnings to existing links and videos, so users will need to mark those manually
## Fixes issue Fixes #493 ## Description of Changes *This change is cherry-picked from OrcaCollective#457 Adds content warnings to links and videos ## Notes for Deployment There's an alembic migration that will need to be run ## Screenshots (if appropriate) New checkbox "Include content warning?" which is checked by default per #493 (comment) ![1](https://github.com/lucyparsons/OpenOversight/assets/66500457/9ea246c7-3f08-48c0-8940-181b676f8c34) How links and videos will look when a content warning is added: ![2](https://github.com/lucyparsons/OpenOversight/assets/66500457/b4d815bc-9919-47f0-ab00-e5fa7bdea024) ## Tests and Linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. --------- Co-authored-by: michplunkett <5885605+michplunkett@users.noreply.github.com>
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 looks great! I've tested the various video types locally and each one looks solid 🚀
const hide = $('<button type="button" class="btn btn-lg">Show video</button>') | ||
hide.click(() => overlay.css("display", "none")) | ||
|
||
const wrapper = $("<div>") |
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.
Do we need to add a </div>
to this or does jquery handle it for us?
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.
Edit: This is the case based on the Jquery documentation!
Description of Changes
Fixes #197.
Adds content warnings to links and videos!
Notes for Deployment
There's an alembic migration that will need to be run
Screenshots (if appropriate)
New checkbox "Include content warning?" which is checked by default per lucyparsons#493 (comment)
How links and videos will look when a content warning is added:
Tests and linting
I have rebased my changes on
main
just lint
passesjust test
passes