fix(external-link): adjust positioning of the icon

[danielhjacobs]

Description

Fix overlapping icons

Motivation

See all issues described in #1776 (comment)

Additional details

N/A

Related issues and pull requests

Fixes #1776

[github-actions]

56cdf09 was deployed to: https://fred-pr1870.review.mdn.allizom.net/

[caugner]

This fixes the menu link, but the icon position in content regresses compared to #1656.

Before	After

[danielhjacobs]

Updated:

[danielhjacobs]

And with inspect element on the live site, used these same CSS styling options (compare to https://developer.mozilla.org/en-US/blog/global-privacy-control/)

[caugner]

Thanks, we're getting there! That said:

• In the menu, the icon is too high compared to the icon in content.
• In the content, the icon is 1-2px too low, and 2-3px too far away from the text.

[danielhjacobs]

In the content, the icon is 1-2px too low, and 2-3px too far away from the text.

I mean, that's the explicit margin-left and the explicit vertical-align. https://developer.mozilla.org/en-US/docs/Web/CSS has an explicit margin-left too, just of .125em instead of .2em, so I'll switch back to that. Meanwhile. that page also, to me, seems to have the exact same vertical alignment provided by using vertical-align: -.125em

[danielhjacobs]

Before:

After:

[caugner]

Hm, unfortunately the vertical-align: -0.125em seems to push the next line further down.

Maybe @pepelsbey has an idea what's going on here, and how to fix it.

[danielhjacobs]

Asked Gemini, got a working alternative: switching from vertical-align: -0.125em to mask-position-y: 0.125em.

[danielhjacobs]

[pepelsbey]

This PR is basically reverting the changes I made to the external link component to fix the new line behavior (when icon doesn't stick to the last word). Just to fix an edge case. Do we really want that?

I don't think we could LLM our way through this. It needs some deeper understanding of the scope and some testing.

[danielhjacobs]

Is there an example page where I can see the icon not sticking to the last word with this change while it does on main? I did inspect element and add l's until a word would have stayed on its own line if the icon were to not be taken into account or end up on the next line if it were and it ended up on the next line as expected.

[pepelsbey]

See the PR that introduced the current behavior.

[danielhjacobs]

I don't see a difference in the rendering of your codepen and my codepen with the changes from this PR:

Your codepen: https://codepen.io/editor/pepelsbey/pen/019e88ad-b90d-71d6-9ec4-798d80d97a33

My codepen: https://codepen.io/editor/danieljacobs/pen/01a086c3-4f9b-7ff2-a066-5ce1cce6fa23

[danielhjacobs]

Notice my codepen switches to:

.external-link-positioned {
	&::after {
		padding-right: 1em;
		margin-left: 0.125em;
		background-color: currentcolor;
		mask-image: var(--external-link);
		mask-position-y: 0.1em;
		mask-size: contain;
		mask-repeat: no-repeat;
		content: "\200b" / " (external)";
	}
}

[danielhjacobs]

[danielhjacobs]

I think the key change from your PR for the fix was the removal of display: inline-block; from ::after, which I kept. Adding that in with my change does revert your PR's changes.

[caugner]

This PR is basically reverting the changes I made to the external link component to fix the new line behavior (when icon doesn't stick to the last word). Just to fix an edge case. Do we really want that?

I was unable to reproduce the issue in this PR, i.e. the icon seems to still be glued to the last word. For me, it looks like the changes are fine and don't cause any regression.

[danielhjacobs]

Before (left) and After (right) for the blog; since the blog preview is not build by CI, I had to inspect element these changes onto the page to observe these differences

[pepelsbey]

Yes, it works in some cases, but not as good as the initial implementation: position: absolute makes it immune to HTML formatting. Compare the rendering of these two links:

<a href="" class="external">Link</a>

<a href="" class="external">
  Link
</a>

The lack of position: aboslute adds a space, which makes the icon to drop again:

Only with the space removed it sticks to the last word:

I don’t think we can guarantee HTML formatting of such links, this is just too fragile. The solution needs to work in both cases: with or without spaces.

[danielhjacobs]

I don’t think we can guarantee HTML formatting of such links

I guess @caugner opened #1885 for that

[danielhjacobs]

Added text-wrap: nowrap to the ::after, fixes the white space issue unless you explicitly add a space before the ending a tag, which seems unlikely.

[danielhjacobs]

None of the links in #1885, even before that PR, have explicit whitespace after the text, making it so this CSS should be sufficient.

[caugner]

It looks like the icon no longer goes in the next line on its own, but the accidental trailing white-space inside the anchor element is now visible in the footer:

Before:

After:

From my side, the current solution is sufficient.

In discussing this with @LeoMcA, we came up with two alternatives: Overriding the external style selectively in other sections, or applying the position: absolute change only inside .content-section.