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.

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

Content flag send selection category #1522

Merged
merged 13 commits into from
Sep 24, 2024
Merged

Conversation

pelumy
Copy link
Contributor

@pelumy pelumy commented Sep 19, 2024

Issues covered

#1499
Part 2 of #1489

Description

This PR includes the functionality to choose how to flag a content in the new moderation flow.

How to test

  1. Build the app
  2. Click the side menu
  3. Click the Settings on the side menu
  4. Scroll down to turn on the toggle for new moderation flow
  5. Go back to the Feed screen by clicking the feed tab at the bottom of the screen
  6. Click on the 3 dots on one of the notes.
  7. Select Flag this content option.
  8. Select an option from the categories displayed.
  9. Please confirm that new categories on how to flag a content shows up under the first category.

Screenshots/Video

Screen.Recording.2024-09-19.at.2.46.08.PM.mov

Copy link
Member

@mplorentz mplorentz left a comment

Choose a reason for hiding this comment

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

I didn't get a chance to do a really thorough review yet but overall this is looking very good! I had one very minor request below. Other than that I think we could merge this (although I see it depends on another PR which still has a requested change).

@pelumy
Copy link
Contributor Author

pelumy commented Sep 20, 2024

I didn't get a chance to do a really thorough review yet but overall this is looking very good! I had one very minor request below. Other than that I think we could merge this (although I see it depends on another PR which still has a requested change).

All requested changes in the previous branch has been made, I didn't want to tag @joshuatbrown today for a final PR since he is OOO.

Base automatically changed from content-flag-category to main September 23, 2024 15:40
Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

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

Overall, this is looking great. I think we could make a couple small UI improvements in addition the inline comment I made.

Simulator Screenshot - iPhone SE (3rd generation) - 2024-09-23 at 18 06 00

  1. The descriptions at the top are being centered but should be left-aligned
  2. There's a little extra space at the bottom of the box that says "The Nos moderation team..."

@@ -3,34 +3,55 @@ import Foundation
/// A model representing a flagging option used in content moderation.
/// - `title`: The title of the flagging option.
/// - `description`: An optional description that provides more detail about the flagging option.
/// - `info`: An optional message that will be displayed when the user has selected a particular flag.
/// - `id`: A unique identifier for the flagging option, based on the `title`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Suggesting we remove the extra line here since we try to keep our doc comments right above the thing they're documenting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alrighty

@pelumy
Copy link
Contributor Author

pelumy commented Sep 24, 2024

Overall, this is looking great. I think we could make a couple small UI improvements in addition the inline comment I made.

Simulator Screenshot - iPhone SE (3rd generation) - 2024-09-23 at 18 06 00

  1. The descriptions at the top are being centered but should be left-aligned
  2. There's a little extra space at the bottom of the box that says "The Nos moderation team..."

@joshuatbrown, figma has a little extra space at the bottom for point 2. Thanks for catching 1, I have added a fix for it.

https://www.figma.com/design/s0qf4VmyQygydP8MIQazZc/Nos?node-id=8505-13706&t=6hmFfcSIzeUDOzt4-4

Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

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

@pelumy you're absolutely right -- there is some extra space at the bottom in Figma. Good catch. This is looking perfect!

@pelumy pelumy added this pull request to the merge queue Sep 24, 2024
Merged via the queue into main with commit 9ca1d49 Sep 24, 2024
4 checks passed
@pelumy pelumy deleted the content-flag-send-to-selection branch September 24, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants