Skip to content
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.

Sign up for GitHub

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

Jump to bottom

In-app content reporting #5808

Merged
merged 5 commits into from
Apr 21, 2021
Merged

In-app content reporting #5808

merged 5 commits into from
Apr 21, 2021

Conversation

infinite-persistence
Copy link
Contributor

@infinite-persistence infinite-persistence commented Apr 1, 2021 •
edited
Loading

Issue

Closes #5676 Content reports (dmca and others)

Screenshots

Here are some screenshots. DMCA one includes more fields, but looks roughly the same.

Click to expand
  • image
  • image
  • image
  • image
  • image
  • image

@tzarebczan
Copy link
Contributor

tzarebczan commented Apr 7, 2021 •
edited
Loading

Will this page accept a URL parameter with claim id? If not, let's please add that. This will be linked from other apps like IOS/Android so the UI doesn't have to be re-created there.

@kauffj
Copy link
Member

kauffj commented Apr 7, 2021

I missed the attached images before. Some feedback:

  • Timestamp UI could be improved if it's easy
  • Colons are almost always a sign that you can improve styling. Properly styled labels and values do not require colons.
  • "Submit Report?" needs another title. Perhaps confirmation dialog could be styled after the confirm publish dialog?
  • The green checkmark next to Report Submitted appears to introduce a new style. Either the dialog should close and a snackbar should be used, or the dialog can stay open and the feedback can be given by the colored bottom bar (I forget the name of this element).

@infinite-persistence infinite-persistence marked this pull request as draft April 8, 2021 10:49
@infinite-persistence infinite-persistence force-pushed the ip/content-report-#5676 branch 4 times, most recently from 7c4f42a to fa68fee Compare April 8, 2021 20:21
@infinite-persistence
Copy link
Contributor Author

infinite-persistence commented Apr 8, 2021 •
edited
Loading

--- Update ---

  • Simplified the timestamp UI. Includes error checking
    image
    image
  • Page is now accessible via /$/report_content?claimId=<claimId>.
  • No more unnecessary colons.
  • Tweaked the confirmation page a bit to look more like the Tip Modal.

Here are some screenshots, this time doing the DMCA path:

📸 Click to expand...
  • image
  • image
  • image
  • image
  • image
  • image

@infinite-persistence infinite-persistence marked this pull request as ready for review April 8, 2021 20:31
neb-b
neb-b reviewed Apr 12, 2021
View reviewed changes
Copy link

@neb-b neb-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small comments but this mostly looks good. This is a big improvement

ui/constants/report_content.js Outdated
export const PARAMETERS = {
type: {
'Sexual content': {
CATEGORIES: [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be [CATEGORIES]

ui/redux/actions/reportContent.js Outdated
type: ACTIONS.REPORT_CONTENT_STARTED,
});

let RICK_URL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does RICK mean?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm. This can probably be something like REPORT_URL

ui/component/reportContent/view.jsx
const str = (n) => n.toLocaleString('en-US', { minimumIntegerDigits: 2, useGrouping: false });
updateInput('timestamp', str(h) + ':' + str(m) + ':' + str(s));
}
}, []);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claimId should be passed to this.

@infinite-persistence
Copy link
Contributor Author

--- Changes ---

  • Update:
    • CATEGORIES to [CATEGORIES]
    • RICK_URL to REPORT_URL
  • Split out the effects, and correct the dependancies.
  • Fix bug of not being able to proceed if reporting on Markdown because timestamp wasn't set.

@infinite-persistence
Copy link
Contributor Author

Ping just in case ... anything else that needs changing?

@tzarebczan
Copy link
Contributor

All good, I'll get this shipped tomorrow. Thanks again!

@neb-b neb-b merged commit cb2c33a into master Apr 21, 2021
@neb-b neb-b deleted the ip/content-report-#5676 branch April 21, 2021 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neb-b neb-b neb-b left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Content reports (dmca and others)
4 participants
@infinite-persistence @tzarebczan @kauffj @neb-b