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

Fix decoloring of comments icon #475

Merged
merged 6 commits into from
May 2, 2023
Merged

Conversation

Splines
Copy link
Member

@Splines Splines commented Apr 24, 2023

@Splines Splines added the bug label Apr 24, 2023
@Splines Splines self-assigned this Apr 24, 2023
@Splines
Copy link
Member Author

Splines commented Apr 24, 2023

In addition to app/views/readers/update_all.coffee and app/views/readers/update.coffee, I also changed these lines in app/views/comments/create.js.erb:

<% if @update_icon %>
$('#commentsIcon').addClass('new-comment');
<% end %>

While doing this, I was wondering if we really want the comments icon to be colored at all when you submit your own comment. Isn't it a bit weird for a user to then click on the icon just to notice it was their own comment 3 seconds ago?

@fosterfarrell9
Copy link
Collaborator

While doing this, I was wondering if we really want the comments icon to be colored at all when you submit your own comment. Isn't it a bit weird for a user to then click on the icon just to notice it was their own comment 3 seconds ago?

We could discuss this. I mean, for me personally, I like to have this coloring even for my own comments, because it gives me some more visual feedback that I did something that other users will notice. Of course, others might disagree with that view.
By the way, for any changes in that direction, #445 should be resolved first, because the code responsible for selecting who gets a coloring is inside the comments_controller.rb in the commontator gem which we currently self-host. It would be better to have this back as a custom controller inside our own repository.

@Splines
Copy link
Member Author

Splines commented Apr 24, 2023

@fosterfarrell9 Of course, that's very subjective. Personally, I want to be notified when another user has made a comment (or even more urgent when somebody else replied to my own comment). For this to work, once I've left a comment, I have to click on the now colorized comments icon and then on either "Clear all" or "Clear" for my own comment. If I don't repeat this for every comment I post, the icon stays colorized and I won't notice that somebody else has posted something.

But we should probably outsource this discussion to an issue (if at all) as this PR is now just intended for this fix ;)

@Splines Splines marked this pull request as ready for review April 24, 2023 16:48
@Splines
Copy link
Member Author

Splines commented Apr 24, 2023

Before squashing, I will look again into the integration tests and find out why they are failing here.

@Splines Splines changed the base branch from main to mampf-next April 28, 2023 14:23
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #475 (47e44fe) into mampf-next (263a355) will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           mampf-next     #475   +/-   ##
===========================================
  Coverage       66.64%   66.64%           
===========================================
  Files             311      311           
  Lines            9363     9363           
===========================================
  Hits             6240     6240           
  Misses           3123     3123           

@Splines Splines merged commit 27394a3 into mampf-next May 2, 2023
@Splines Splines deleted the fix/comments-button-reset-color branch May 2, 2023 15:38
@Splines Splines mentioned this pull request May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decoloring of commentsIcon does not work
2 participants