Optimize profile layout to enhance visual experience

[kerwin612]

before:

after:

[silverwind]

Hmm I can't reproduce your "before", it already looks good to me:

If it really is an issue, I think this one may be better solved with some CSS because there is already CSS targeting these <li> elements.

[kerwin612]

Hmm I can't reproduce your "before", it already looks good to me:

If it really is an issue, I think this one may be better solved with some CSS because there is already CSS targeting these `

` elements.

[kerwin612]

The reason for this problem is that fixed-size svg is not centered between its two parent containers;

[silverwind]

Looks fine for me in latest Firefox and Chrome:

Which browser are you using?

[kerwin612]

My solution is to add flex&center to both parent containers. Having a fixed style ensures that it is centered.

[silverwind]

@lunny why set that label when there is only 1 review and we aren't even sure whether this is valid at all?

[lunny]

@lunny why set that label when there is only 1 review and we aren't even sure whether this is valid at all?

Sorry, made a mistake.

[kerwin612]

@lunny why set that label when there is only 1 review and we aren't even sure whether this is valid at all?

Sorry, made a mistake.

Upon careful review of our previous communications, it can be seen that:

Firstly, I encountered this issue on my Edge browser: the icons and their corresponding text are not horizontally centered;

Subsequently, I pinpointed the cause by tracing back: there are two parent containers above the icon, neither of which had the display property set, resulting in their child elements not being centered. The height of the parent is 20, the child's height is 19, and the icon's height is 16;

Finally, I resolved this issue by applying flex and center styles to both parent containers; As a result, I naturally proceeded to submit this pull request (PR).

As for @silverwind , he states that the issue does not exist on his Firefox and Chrome browsers, but his evidence does not negate the problem I've raised! The issue I've brought up at least proves that there is indeed a style anomaly here!

Moreover, my modifications have indeed resolved the issue I raised, and have not introduced any new problems. So, what else do I need to do?

@lunny

[yp05327]

I can reproduce this in Windows's Chrome and Edge.
And I also checked this in macOS's Chrome, the behaviour is different:

edited: maybe not correct

in Windows's Chrome:

in macOS's Chrome:

edit:
maybe this is because of the screen size and the scaling, not sure about it.
but what I want to say is that, this may be caused by different OS

[silverwind]

Will check later whether there is a better way.

[MrSuddenJoy]

This code looks good to me. Well done 👍🏻

[kerwin612]

Okay, even if I can not reproduce on MacOS, I think there must be a better way than littering the code with flex-text-inline, and even nesting it.

I have identified the issue and resolved it; thus, at the very least, this issue indeed needed to be addressed. If you believe my modification is not reasonable enough, please state your thoughts, and I will cooperate to make corrections, rather than outright rejecting the modification.

[lunny]

Will check later whether there is a better way.

@kerwin612 Please wait for silverwind's review.

[silverwind]

Pushed a fix that removed the unnecessary and unsemantic <i> element altogether and with that, the nesting of flex-text-inline is gone.

[wxiaoguang]

Hmm, it looks like an abuse.

The flex and align items should be on the parent element <li> to align the svg mail / mailto / svg private.

No idea why it is only on the svg private .....

[kerwin612]

As in the screenshot, the li element has already been styled appropriately. The current Pull Request resolves the issue where the svg element inside the a element is not centered.

[silverwind]

Yes I agree flex-text-inline is likely not ideal, but it's aiming at this specific alignment problem that for some reason only shows on Windows, it does not reproduce on any MacOS browser for me.

[wxiaoguang]

I guess the root problem is the "spaces" between elements, which make the height different.

I guess using {{- svg ... -}} would make the height correct and no need to use "flex" on "a". Just a guess and using "flex" is still acceptable.

[silverwind]

Yes, likely it is whitespace-related because the display: flex removes the whitespace rendering.