Skip to content

Conversation

@eth3lbert
Copy link
Contributor

A report form will display in a dialog when the report button is clicked.
Users must select reasons and provide detail information (required when the "other" reason is chosen) before submitting.

Closes #9478

Screenshots:

image

image

image

@codecov
Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.13%. Comparing base (bd1be54) to head (0c6a1ae).
Report is 76 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9496      +/-   ##
==========================================
+ Coverage   89.10%   89.13%   +0.02%     
==========================================
  Files         286      285       -1     
  Lines       29039    29026      -13     
==========================================
- Hits        25876    25872       -4     
+ Misses       3163     3154       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

A report form will display in a dialog when the report button is clicked.
Users must select reasons and provide detail information (required when
the "other" reason is chosen) before submitting.
@eth3lbert eth3lbert force-pushed the krate-report-form branch 3 times, most recently from 39f6111 to f67bb18 Compare September 23, 2024 16:56
@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-frontend 🐹 labels Sep 23, 2024
@Turbo87
Copy link
Member

Turbo87 commented Sep 23, 2024

sweet, thank you! I was about to start on something similar :D

looks mostly good to me, but I'm wondering about the decision to use a dialog. I think I would've used a fullscreen route like /crates/foo/report instead. do you see any advantages of using a dialog instead?

@eth3lbert
Copy link
Contributor Author

looks mostly good to me, but I'm wondering about the decision to use a dialog. I think I would've used a fullscreen route like /crates/foo/report instead. do you see any advantages of using a dialog instead?

Actually, a route would also work and probably fits better with crates.io's design since we don't currently rely on modals UI. However, I also think this relatively simple operation would work with a dialog, which has the benefit of not interrupting the current workflow and avoids navigating to another location. However, some users might have trouble with a dialog on certain browsers(?). Ultimately, I think both are valid options, and I don't have a strong preference, I'm also Ok report in a new page if it's more suitable 😉 .

@eth3lbert
Copy link
Contributor Author

I'm also unsure what caused the UI difference in the README rendering test (I don't know how to set up local README rendering 🙈 ). This issue might also be resolved with a new page implementation.

@Turbo87
Copy link
Member

Turbo87 commented Sep 24, 2024

some users might have trouble with a dialog on certain browsers

I was mostly worried about the UX on mobile. it's possible to make dialogs work decently on mobile devices, but from my experience it's quite hard, which is why I tend to avoid dialogs if possible :D

the report button was somewhat inspired by the one on npmjs.com. there it links to something like https://www.npmjs.com/support?inquire=security&security-inquire=abuse-or-spam, which is also a full-page contact form.

I'm also unsure what cauxsed the UI difference in the README rendering test

it seems that we have some sort of race condition between our Percy snapshots and the mermaid diagram rendering, though I've only noticed it on the playwright test suite so far. we might need another instruction to wait for the diagram to finish rendering before we take the snapshot.

@eth3lbert
Copy link
Contributor Author

I was mostly worried about the UX on mobile. it's possible to make dialogs work decently on mobile devices, but from my experience it's quite hard, which is why I tend to avoid dialogs if possible :D

Fair enough!

the report button was somewhat inspired by the one on npmjs.com. there it links to something like https://www.npmjs.com/support?inquire=security&security-inquire=abuse-or-spam, which is also a full-page contact form.

Yeah, this is a great one that we can learn from.
I guess we just want to implement something similar to the current one, but on a full page? Or do you lean toward implementing it as a general report page?

it seems that we have some sort of race condition between our Percy snapshots and the mermaid diagram rendering, though I've only noticed it on the playwright test suite so far. we might need another instruction to wait for the diagram to finish rendering before we take the snapshot.

Oh, that's interesting. I haven't encountered this issue before. Thanks for the information.

@Turbo87
Copy link
Member

Turbo87 commented Sep 24, 2024

I guess we just want to implement something similar to the current one, but on a full page? Or do you lean toward implementing it as a general report page?

I'm undecided on this one. What do you think? It's probably easiest to start with the former and incrementally evolve it into the latter?

We also still have #7081 open, which could be solved by either having another dedicated "Report User" page, or make it an option on the general report page.

@eth3lbert
Copy link
Contributor Author

eth3lbert commented Sep 24, 2024

I'm undecided on this one. What do you think? It's probably easiest to start with the former and incrementally evolve it into the latter?

We also still have #7081 open, which could be solved by either having another dedicated "Report User" page, or make it an option on the general report page.

Then I lean toward a more general contact support page, starting with simple options (like a single option "Report a crate or user") and gradually expanding them later.

Do we already have a list of the support services we offer? (We might need feedback from others who handle support.)

@Turbo87
Copy link
Member

Turbo87 commented Sep 24, 2024

Do we already have a list of the support services we offer?

not really. we could take a look at the support requests that we've received and handled in the past, but we would probably need some sort of generic option anyway. the most common request is from author who would like to have their crates deleted for various reasons, but that should be covered by the crate deletion RFC implementation "soon".

@eth3lbert
Copy link
Contributor Author

@Turbo87 I have experimented full-page fashion, and most flows follow npm. I'll try to open a PR later sometime.

localhost_4200_support

localhost_4200_support_inquire=crate-violation

The designs and wording need further refinement in that PR.

@eth3lbert eth3lbert mentioned this pull request Sep 27, 2024
@Turbo87
Copy link
Member

Turbo87 commented Oct 1, 2024

superseded by #9529

@Turbo87 Turbo87 closed this Oct 1, 2024
@bors
Copy link
Contributor

bors commented Oct 1, 2024

☔ The latest upstream changes (presumably #9529) made this pull request unmergeable. Please resolve the merge conflicts.

@eth3lbert eth3lbert deleted the krate-report-form branch October 1, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-frontend 🐹 C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Report a crate" button doesn't require any information

3 participants