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

Ability to report comments. #816

Merged
merged 12 commits into from
Oct 17, 2023
Merged

Ability to report comments. #816

merged 12 commits into from
Oct 17, 2023

Conversation

ggichure
Copy link
Collaborator

@ggichure ggichure commented Oct 10, 2023 •
edited
Loading

Pull Request Description

This PR Adds ability to report comments.

Issue Being Fixed

Issue Number: #252

Screenshots / Recordings

report_comments.webm

For Errors

Screenshot 2023-10-12 at 19 29 59

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

@ggichure ggichure marked this pull request as draft October 10, 2023 16:31
@ggichure ggichure changed the title Init report comment. Ability to report comments. Oct 10, 2023
@ggichure ggichure marked this pull request as ready for review October 11, 2023 07:23
@ggichure ggichure marked this pull request as draft October 11, 2023 08:10
@ggichure ggichure marked this pull request as ready for review October 12, 2023 16:33
micahmo
micahmo reviewed Oct 12, 2023
View reviewed changes
Copy link
Member

@micahmo micahmo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work and nice feature!! And thanks for the additional localizations that you added!

I left some comments for discussion.

I also had one visual suggestion. When you press report, the button disappears and a progress indicator appears. The progress indicator is both taller and narrower than the button, which means the UI awkwardly shifts. What do you think about integrating it like this?

qemu-system-x86_64_xI0WUOq9WO.mp4

lib/post/bloc/post_bloc.dart Outdated Show resolved Hide resolved
lib/post/bloc/post_bloc.dart Show resolved Hide resolved
lib/post/utils/comment_action_helpers.dart Outdated Show resolved Hide resolved
lib/post/utils/comment_action_helpers.dart Outdated Show resolved Hide resolved
lib/post/widgets/comment_card.dart Outdated Show resolved Hide resolved
lib/user/pages/user_page_success.dart Show resolved Hide resolved
@ggichure
Copy link
Collaborator Author

Nice work and nice feature!! And thanks for the additional localizations that you added!

I left some comments for discussion.

I also had one visual suggestion. When you press report, the button disappears and a progress indicator appears. The progress indicator is both taller and narrower than the button, which means the UI awkwardly shifts. What do you think about integrating it like this?
qemu-system-x86_64_xI0WUOq9WO.mp4

Nice. I'll work on the progress indicator.

@hjiangsu
Copy link
Member

Just double checking that this is ready for review?

@ggichure
Copy link
Collaborator Author

Just double checking that this is ready for review?

Yes it is.

hjiangsu
hjiangsu approved these changes Oct 17, 2023
View reviewed changes
Copy link
Member

@hjiangsu hjiangsu left a comment

Choose a reason for hiding this comment

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

LGTM!

@hjiangsu hjiangsu merged commit 95101d5 into thunder-app:develop Oct 17, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@micahmo micahmo micahmo left review comments

@hjiangsu hjiangsu hjiangsu approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@ggichure @hjiangsu @micahmo