Fix cursor for avatar and user names in comments #7256
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danxuliu
added
2. developing
Codecov Report
@@ Coverage Diff @@
## master #7256 +/- ##
============================================
- Coverage 50.85% 50.75% -0.11%
- Complexity 24551 24622 +71
============================================
Files 1585 1585
Lines 93833 94046 +213
Branches 1354 1356 +2
============================================
+ Hits 47719 47731 +12
- Misses 46114 46315 +201
|
Member
|
@danxuliu yeah, we can remove the menu for comments being composed! :) |
When it was on an author row the cursor was shown as a pointer, even if clicking on the author row itself does nothing. On the other hand, avatars used the default cursor, even if clicking on them either shows the contacts menu (in the case of the author row, only when the avatar is for a different user than the current one) or inserts a mention (for avatars shown in the list of suggested mentions), depending on the case. Now, the author row uses the default cursor, and avatars (and their associated user name) use a pointer cursor if clicking on them will trigger an action (either showing the contacts menu or inserting a mention). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The contacts menu does not provide too much value for users mentioned in a message being composed, so it is now disabled in this case. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The contacts menu is not shown for avatars and user names in the author row if they represent the current user. For consistency, and because the contacts menu provides no value when shown for the current user, this commit also disables the contacts menu for mentions to the current user. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
force-pushed
the
fix-cursor-for-avatar-and-user-names-in-comments
branch
from
f83a6ca to
ea4414f
Compare
Member
Author
Done! I have also disabled the contacts menu for mentions to the current user in final messages. Also, after disabling the contacts menu for comments being composed and with some changes I made to the original CSS selectors this pull request no longer depends on what is decided in #7254; either way the contacts menu and the cursor will work as expected :-) (or at least I think so :-P ) With all that this pull request is now ready to be reviewed! |
danxuliu
added
3. to review
apps/comments/js/commentstabview.js
Outdated
| + ' data-user-display-name="' | ||
| + _.escape(displayName) + '"></div>'; | ||
|
|
||
| var isCurrentUser = (uid === oc_current_user); |
Member
There was a problem hiding this comment.
oc_current_user is deprecated?
Member
There was a problem hiding this comment.
Let me replace that with OC.getCurrentUser().uid
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Member
Author
|
(Yes, I am late to the party, but anyway) Oops, I did not know that @LukasReschke Your fix was perfect, thanks :-) |
danxuliu
added a commit
that referenced
this pull request
Nov 28, 2017
Although #7256 was merged cleanly some of the changes really conflicted with those introduced by the last commit of #7251, and this broke the appearance of the author row of comments. This commit fixes those silent conflicts and restores the appearance of the author row. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
|
|
||
| $.merge(avatar, strong).contactsMenu(avatar.data('user'), 0, appendTo); | ||
| var username = $(this).data('username'); | ||
| if (username !== oc_current_user) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When it was on an author row the cursor was shown as a pointer, even if clicking on the author row itself does nothing. Also avatars used the default cursor, even if clicking on them shows the contacts menu (if the avatar is for a different user than the current one). Now the cursor is shown as a pointer on avatar and user names of users different than the current one.
There are still some changes pending based on what is decided in #7254 (right now the cursor used for mentions in messages being composed is the default one; changing it to a pointer would require removing
.commentfrom#commentsTabView .comments .message...and also adding thecurrentUserclass to the appropriate mentions through JavaScript).@jancborchardt Besides that maybe the contacts menu should be completely disabled for mentions in messages being composed, as it does not provide too much value there (and, moreover, right now it does not appear in the proper position).