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

Instantly display the 'You reported the X' message #15299

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Instantly display the 'You reported the X' message #15299

merged 1 commit into from
Dec 4, 2023

Conversation

dmarcoux
Copy link
Contributor

A page refresh isn't needed anymore for the message to appear.

For comments, the implementation isn't really robust, but considering how the comments are rendered, this is what we can do without refactoring a lot of code. In the best scenario, rendering a comment would be as simple as rendering a view component. It would then be unneccessary to fiddle much with JavaScript, re-rendering the comment view component would be the solution.

As for projects and packages, it's not possible to re-render the side_links partials since they depend upon so many objects and we don't have those in webui/reports_controller.

@github-actions github-actions bot added the Frontend Things related to the OBS RoR app label Nov 30, 2023
@dmarcoux
Copy link
Contributor Author

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #15299 (e30abe9) into master (cdbaec8) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #15299   +/-   ##
=======================================
  Coverage   87.76%   87.76%           
=======================================
  Files         778      778           
  Lines       25576    25576           
=======================================
  Hits        22448    22448           
  Misses       3128     3128           

@dmarcoux dmarcoux marked this pull request as ready for review December 1, 2023 13:24
@dmarcoux dmarcoux added the review-app Apply this label if you want a review app started label Dec 1, 2023
@obs-bot
Copy link
Collaborator

obs-bot commented Dec 1, 2023

Review app will appear here: http://obs-reviewlab.opensuse.org/dmarcoux-report-ux

@saraycp
Copy link
Contributor

saraycp commented Dec 1, 2023

There is one case missing: reporting a comment on a Request (redesigned page) requires a refresh to see the yellow text about the report inside the bubble.

Compare these two comment bubbles. This is what I did:

  • I reported the first comment and then refreshed the page => yellow text is there
  • I reported the second comment but didn't refresh afterwards => no yellow text.

Screenshot 2023-12-01 at 15-02-24 Open Build Service

@dmarcoux
Copy link
Contributor Author

dmarcoux commented Dec 1, 2023

There is one case missing: reporting a comment on a Request (redesigned page) requires a refresh to see the yellow text about the report inside the bubble.

Oh right, I forgot that. The comments are different there... I'll have a look.

function showYouReportedMessage(reportLinkId, reportableType, reportableId, message) {
switch(reportableType) {
case 'Comment':
// Comments differ depending on where they are, so this is why we have two ways. If an element isn't found, nothing will happen...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't say this is great. As I wrote in the PR description, given how the comments are rendered, I decided to do it this way.

@saraycp
Copy link
Contributor

saraycp commented Dec 1, 2023

@dmarcoux could you please get rid of the white background behind the flag icon? (in the yellow text).

It seems it's somehow inherited but not needed in the places where the text is displayed.

Copy link
Contributor

@saraycp saraycp left a comment

Choose a reason for hiding this comment

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

I leave my approval. Only missing the change of the background color.

A page refresh isn't needed anymore for the message to appear.

For comments, the implementation isn't really robust, but considering
how the comments are rendered, this is what we can do without
refactoring a lot of code. In the best scenario, rendering a comment
would be as simple as rendering a single view component. It would then
be unneccessary to fiddle much with JavaScript, re-rendering the comment
view component would be the solution.

As for projects and packages, it's not possible to re-render the
side_links partials since they depend upon so many objects and we don't
have those in webui/reports_controller.
@krauselukas krauselukas merged commit d6b771b into openSUSE:master Dec 4, 2023
19 of 20 checks passed
@dmarcoux dmarcoux deleted the report-ux branch December 7, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Things related to the OBS RoR app review-app Apply this label if you want a review app started
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants