Fixed contact menu position on comments

[skjnldsv]

Fix #7250

The popover needs a fix too for the activity app, but this needs to be done on the activity repo.

[codecov]

Codecov Report

Merging #7251 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #7251      +/-   ##
============================================
- Coverage     50.86%   50.86%   -0.01%     
  Complexity    24551    24551              
============================================
  Files          1585     1585              
  Lines         93815    93815              
  Branches       1354     1354              
============================================
- Hits          47720    47716       -4     
- Misses        46095    46099       +4

Impacted Files	Coverage Δ	Complexity Δ
core/js/js.js	62.98% <0%> (-0.57%)	0% <0%> (ø)
apps/files_trashbin/lib/Trashbin.php	72.53% <0%> (+0.24%)	136% <0%> (ø)	⬇️

[danxuliu]

Unfortunately setting the top CSS attribute fixed the contacts menu when shown for a comment author, but moved it when shown for a mention in the text:

Author:

Mention:

I have investigated the issue and it turns out that the commit 2348d14 did not break the position; the vertical position was already wrong, that commit just made it more evident ;-)

The real cause was displaying the child divs of .authorRow as inline-block; this caused the .contactsmenu-popover to be displayed too as inline-block, which in turn caused the top of the contacts menu to be aligned to the top of the author row. Before the commit 2348d14 .menu elements had a top: 45px rule, which moved the contacts menu to the lower part of the author row (in fact it was too low), and when that commit removed the general CSS rules for .menu by assigning them to #header .menu then there was no vertical displacement for the contacts menu and thus it appeared aligned to the top of the author row.

I must admit, however, that I do not understand why displaying the contacts menu popover as a block aligns it to the center of the avatar (I can not see any top: 32px rule applied to the contacts menu), but it works :-P , so if someone could clarify that it would be great ;-)

Anyway I took the liberty to amend your commit to just fix the horizontal position and add another commit to fix the vertical position by displaying the contacts menu as a block; I did that using the :not selector, but I do not know if the preferred approach is to use that selector or just add a different, more specific rule that reverts the general one (#commentsTabView .authorRow>div.contactsmenu-popover { display: block; }).

[skjnldsv]

@danxuliu Sorry I did not realised there was a popover for the main avatar as well :)

[skjnldsv]

@danxuliu there, I fixed most of the stuff.
We definitely needs to standardize the sidebars, this is a mess! :'(
cc @jancborchardt

[pixelipo]

Yes, @skjnldsv - check issue #7255

[pixelipo]

I'm not really sure what causes these errors, but they should be fixed.

[pixelipo]

I think this should be applied to .avatar-name-wrapper instead.

[danxuliu]

@skjnldsv

Sorry I did not realised there was a popover for the main avatar as well :)

No problem, I should have mentioned it in the original issue ;-)

there, I fixed most of the stuff.

Thanks :-) But... can I ask what was wrong with the fix that I pushed to this branch?

Also, from what I see with your new changes the tip of the arrow of the tooltip is no longer at the centre of the avatar, but a little to the right and to the bottom (although maybe that is intended, I do not know).

[skjnldsv]

But... can I ask what was wrong with the fix that I pushed to this branch?

I'm an idiot, I overwrite it with my push! 🙈

[danxuliu]

I'm an idiot, I overwrite it with my push!

xD To be honest I thought it was something like that :-P Do not worry; I can rebase your new changes on mines or push to a different branch so you can compare. As you prefer ;-)

[skjnldsv]

@danxuliu don't mock me! :p
Do as you please, forgive me :)

[danxuliu]

@skjnldsv

Do as you please, forgive me :)

You have my blessing :-P

I have rebased your last commit onto my previous changes; due to my changes it seems that your last commit is not needed to fix the contact menu position, but I prefer you to check it. Also, even if it was not needed to fix the position it may cover other cases I am not aware of, or simply enhance the CSS rules (I am aware of but not very familiar with flex displays, so I do not know its implications here). In any case the commit message should be reworded ;-)

Besides all that, from what I can see the tip of the arrow is slightly moved to the right and to the bottom. It is caused by left: -6px instead of left: -7px and margin-top: 0 in .contactsmenu-popover in last commit, but as I said maybe it is an explicit design decision, I do not know.

@pixelipo
Could you provide a screenshot with a bit more context (I mean, not so focused in the problem)? I do not know what I am seeing :-)

[nickvergessen]

Works

[MorrisJobke]

Tested and works 👍