Add a more detailed modal when reporting a post or a comment #8035
Conversation
Flaburgan
force-pushed
the
improve-report-form
branch
from
1d7bf79
to
cc23999
Compare
If your intention of this PR is to address this issue, then it won't help. Mainly because those who file those reports also would not read the modal. If you want to get people to report only valid things, then you have to go the Facebook/Twitter route and have them select from a list of predefined reason, some of which end in a dialog explaining how to block people, while others actually do submit a report. |
|
That's a good idea, but rise the question about how to make that generic for all pods... It means we could suggest a list the podmin would be able to edit, I guess. |
|
I actually have the same question, but for your PR. Some pods, for example Geraspora, do not just refer to the ToS, but also to the project's community guidelines for content policing. Makes me wonder if we should expose that text in the config, but that would kill all chance of localizing that. Hm. |
Flaburgan
force-pushed
the
improve-report-form
branch
2 times, most recently
from
a2db434
to
301d342
Compare
There was a problem hiding this comment.
I have reconsidered my opinion here. I think referring to the terms is fine, as podmins can adjust their ToS templates to include links to additional documents like the Community Guidelines.
Some comments. I'm not really a fan of the current wording, and will propose an alternative once I had a bit more time to consider that. :)
|
Bump on conflicting file |
Flaburgan
force-pushed
the
improve-report-form
branch
from
301d342
to
9cb21b1
Compare
|
I rebased this PR, but I also marked it as Draft because of #8035 (comment) if someone can explain this to me (@jhass maybe?) |
Flaburgan
force-pushed
the
improve-report-form
branch
from
9cb21b1
to
7bb266d
Compare
Flaburgan
changed the title
|
@Flaburgan Can you check how this will show in mobile view? The Issue "Reporting in mobile" is yelled several times and I also think a mobile 'reporting' is very useful. |
|
Un-requesting my own review here. I won't have the time to properly investigate this anytime soon, and I don't want to give the impression. <3 |
The "report" feature doesn't exist at all in the mobile version, it would need to be added from scratch. I would say this work is out of scope of that PR and will be tracked in #8281
Sure no problem. Take care 😉 |
Flaburgan
force-pushed
the
improve-report-form
branch
from
7bb266d
to
f5e4365
Compare
Flaburgan
force-pushed
the
improve-report-form
branch
from
f5e4365
to
5478c98
Compare
It was an
Yeah indeed. I saw that but I was wondering if that was a good point for people reporting several posts in a row, but I agree this is going the other way that what I am trying to achieve here, and can be disturbing. I'm going to fix that.
There is no default as we want to force the user to fill something. It could have a placeholder though, I'm going to add one.
Ok. |
|
Okay I think I fixed everything please tell me if this is now fine. |
Flaburgan
force-pushed
the
improve-report-form
branch
from
ef2c7aa
to
2ed77de
Compare
|
@SuperTux88 ok so code wise we should be good to go. Jasmine is still hanging sometimes, I think @denschub investigated this and his suggestion was to remove the problematic test? I don't remember and I don't know where his commit is. |
app/views/report/_report_modal.erb
Outdated
| <p><%= t("report.modal.you_can_block") %></p> | ||
| <form id="report-content-form" accept-charset="UTF-8"> | ||
| <p><label for="report-reason-field"><%= t("report.modal.reason_label") %></label></p> | ||
| <p><textarea id="report-reason-field" class="form-control" placeholder="<%= t('report.modal.placeholder') %>" autofocus required></textarea></p> |
There was a problem hiding this comment.
I don't think it really is important actually.
app/views/report/_report_modal.erb
Outdated
| <p><%= t("report.modal.you_can_block") %></p> | ||
| <form id="report-content-form" accept-charset="UTF-8"> | ||
| <p><label for="report-reason-field"><%= t("report.modal.reason_label") %></label></p> | ||
| <p><textarea id="report-reason-field" class="form-control" placeholder="<%= t('report.modal.placeholder') %>" autofocus required></textarea></p> |
There was a problem hiding this comment.
I think the whole interface can be improved, but because the work on this already started in #8239 I don't think I should change it in this MR, it will create conflicts. I think it's fine to merge it even if everything is displayed on one line, it doesn't break the UI:
If you disagree then I would suggest to have only one merge request for this and #8239 so that we work on the same branch and are not overlapping the work.
|
@Flaburgan I have some days off, so I can make a fresh rebase of #8239 to this one- (But no bigger code improvements) |
Yes, see #8360. The broken test is and it's re-enabled because you force-pushed my commit away. :D You can re-disable the test, simply by replacing the |
Aaaaah sorry, indeed. I found back your commit 840478b but even after fetching your fork I was not able to cherry-pick it. That's fine I just rewrote it. So, CI should be green soon, fingers crossed. Do we want to disable the display of Jasmine output that we are introducing here or do we want to keep it? |
Flaburgan
force-pushed
the
improve-report-form
branch
from
aa2b7fe
to
dd31a35
Compare
That commit was only for debugging and should be removed, same for the increase of the timeout (Both were already removed, but you brought them back when you removed the commit that disabled the test). |
|
Ok I will remove them. And d63e3b9 ? Not sure how useful it is in the end... |
|
If it works without, it's not useful. :) Our test suite doesn't do a .. god job at maintaining a consistent state for each test, so it might be possible that that's needed, but I'd try without first. And if it's needed, please file a new bug to fix the tests that do weird things. |
Oh yeah, that was already removed again too. So if it works without it, it can be removed too. |
Flaburgan
force-pushed
the
improve-report-form
branch
from
dd31a35
to
953023c
Compare
|
ok, everything should be fine now. Only question is, do you agree with #8035 (comment) ? |
app/views/report/_report_modal.erb
Outdated
| <p><%= t("report.modal.you_can_block") %></p> | ||
| <form id="report-content-form" accept-charset="UTF-8"> | ||
| <p><label for="report-reason-field"><%= t("report.modal.reason_label") %></label></p> | ||
| <p><textarea id="report-reason-field" class="form-control" placeholder="<%= t('report.modal.placeholder') %>" autofocus required></textarea></p> |
There was a problem hiding this comment.
Well, it needs to be fixed somewhere, and I would have thought it fits better to this PR, because as soon as this is merged people are probably starting to send multi-line reports which then aren't rendered correctly. But if you rather want to fix it in the other PR, that's fine for me too, as long as both are in the same release (so 1.0 probably). But the current state is pretty "broken".
app/views/report/_report_modal.erb
Outdated
| <p><%= t("report.modal.you_can_block") %></p> | ||
| <form id="report-content-form" accept-charset="UTF-8"> | ||
| <p><label for="report-reason-field"><%= t("report.modal.reason_label") %></label></p> | ||
| <p><textarea id="report-reason-field" class="form-control" placeholder="<%= t('report.modal.placeholder') %>" autofocus required></textarea></p> |
There was a problem hiding this comment.
But if it's not working, it can also be removed again?
And it might not be super important, but it's still nice to have, and I think it shouldn't be hard to set focus with javascript when opening the modal?
Flaburgan
force-pushed
the
improve-report-form
branch
2 times, most recently
from
aa5289f
to
50f3674
Compare
Flaburgan
force-pushed
the
improve-report-form
branch
from
50f3674
to
7ba9447
Compare
Flaburgan
force-pushed
the
improve-report-form
branch
from
7ba9447
to
ce62d79
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Flaburgan
force-pushed
the
improve-report-form
branch
from
8353654
to
4ff3cb3
Compare
Flaburgan
force-pushed
the
improve-report-form
branch
from
4ff3cb3
to
bef7eb6
Compare
|
Everything is rebased and tests are green. @SuperTux88 if you can review it again, and once it's merged I can switch on #8239 |
SuperTux88
force-pushed
the
improve-report-form
branch
from
bef7eb6
to
587e106
Compare
There was a problem hiding this comment.
Removed some extra newlines myself, otherwise looks good. Assuming you'll fix #8035 (comment) in #8239
As a moderator,
I want to receive report only for posts and comments which violate the terms of use
To easily focus on the important ones
Many reports received on pods I moderate are a people whining about others "look how bad they talk to me". As a consequence of that flow, important reports are sometimes lost in the middle of this. As an attempt to reduce "not real report", I implemented a real modal (instead of the previous javascript alert) which tell to the users to report only content which violates the terms of use, and remind them that they can block the other user if they want to. Also, the reason is required but not filled, which force them to fill something.
Here is how it currently looks like: