Change 836318 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] Link: Make selectors flexible

https://gerrit.wikimedia.org/r/836318

Change 829238 merged by jenkins-bot:

[design/codex@main] Link: implement Codex link styles as Less mixins

https://gerrit.wikimedia.org/r/829238

Change 836318 merged by jenkins-bot:

[design/codex@main] Link: Make selectors flexible

https://gerrit.wikimedia.org/r/836318

@egardner see this patch; once it's merged we can move this task into design review!

Change 849191 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update Codex from v0.2.1 to v0.2.2

https://gerrit.wikimedia.org/r/849191

Change 849191 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.2.1 to v0.2.2

https://gerrit.wikimedia.org/r/849191

Design sign-off done. Some things we need to fix:

1. Could we add a configurable demo for the Link component? We should test some things like:
   • With/without start icon (and change the start icon)
   • With/without external link icon
   • Default / Underlined
   • With different font sizes and formats (Regular, Bold)

2. Visited focus border should be blue (now it's yellow)

Figma spec	Demo (Base link)	Demo (Red link)

3. Red link focus should use border-color-progressive--focus since this red link it's not an error but a new page link

Figma spec	Demo

4. External link should be 12px size (start icon is 20px but external link icon should always be 12px since it needs to be smaller).

5. External link icon should be the one created in this task (I can't find it in the Codex icons, I think we need to add the new external link icon there)

6. A 4px padding is needed between icon and text (for both startIcon-text and text-externalLink)

7. Focus border is irregular when the link has start or external link icon

8. Keyboard navigation is not working in Safari

Thanks @bmartinezcalvo!
One comment, we don't feature link icons in our environment before the link text in paragraphs.
Icons in links in running text are only used as addendum in so far is the first example in the demo misleading.

In T316895#8346505, @Volker_E wrote:

Thanks @bmartinezcalvo!
One comment, we don't feature link icons in our environment before the link text in paragraphs.
Icons in links in running text are only used as addendum in so far is the first example in the demo misleading.

@Volker_E right, this was documented as a misuse in the Figma spec sheet and we should document it in the Codex demo too. It would be great if we can add a recommendations section with some images in our Codex demos.

The distorted focus outline is a browser decision that we shouldn't touch. It's very common in interfaces all around and the exception and the only way out would be to try to get padding there, which has other side-effects like having running text distortion effects at times. Accepted weirdness.

Safari tabbed navigation is, ahem different, by default. We can ignore this as browser quirks. If you want to you can make it behave as every other more useful browser.

Change 850039 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] Link: Apply design fixes

https://gerrit.wikimedia.org/r/850039

Is the yellow focus border a browser-specific thing? I'm not seeing it in FF, Safari, or Chrome on my Mac.

I agree with @Volker_E that there are certain elements of browser-defined behavior we will probably have to live with.

In T316895#8350666, @egardner wrote:

Is the yellow focus border a browser-specific thing? I'm not seeing it in FF, Safari, or Chrome on my Mac.

I agree with @Volker_E that there are certain elements of browser-defined behavior we will probably have to live with.

@egardner sorry, it was my error. I was checking the component forcing the element state with the inspector.

The visited focus is ok so I've removed the item from the design review list.

Would it make sense to attempt to provide a little bit of extra outline-offset in the focus styles of Links that contain icons?

Compare this (default):

With this (2px outline-offset added):

Personally I think the latter is better – a little bit of extra breathing room looks nicer to my eyes. However, I assume we'd only want this rule applied in cases where a .cdx-icon element sits inside of a link as a direct child. Traditionally there has not been a good way in CSS to write these sorts of rules, where you need to change the styles of the parent based on the existence of a child.

The new CSS :has() pseudo-class gives us the ability to do this. You could write a rule like this to ensure that the extra outline offset only got applied when an Icon was present inside a link:

.cdx-mixin-link-base() {
    &:has( > .cdx-icon ) {
        &:focus {
            outline-offset: 2px;
        }
    }
}

Browser support for :has() is limited, but I think we could treat this as a progressive enhancement – browsers that don't support :has() would see unchanged behavior, but modern that do support this feature would get something slightly nicer.

Thoughts?

I wouldn't care too much for two reasons:

• outline-offset isn't the magic bullet in my opinion. We might extend the offset but then run into issues in languages with very tight word spacings like in Burmese. We also shouldn't just do that for Codex interfaces. If that's evaluated appropriately, let's do it on a MediaWiki/WM skin level and in Codex

• the only icon usage in current links is added meta information about the type of the link coming in a smaller than usual icon size (12px at 14px base; slightly more at 16px base), so the current outline quirks disappears in the upcoming change

@bmartinezcalvo a strict 12px target for the external link icon would be too restrictive in our current 14/16 base font mix.
Please compare the updated patch.

Also, the different external link icon on smaller size is a) an overhead, that I still would like to finally discuss. We also need to implement it if agreed on in a different task, Link shouldn't be blocked by it.

In T316895#8351652, @Volker_E wrote:

@bmartinezcalvo a strict 12px target for the external link icon would be too restrictive in our current 14/16 base font mix.
Please compare the updated patch.

@Volker_E if we use the old icon 12px size it's too thin, but if we use the new icon instead this 12px icon size would be perfect for an external icon next to a 14/16px text as specified in the Figma spec.

Also, the different external link icon on smaller size is a) an overhead, that I still would like to finally discuss. We also need to implement it if agreed on in a different task, Link shouldn't be blocked by it.

In this task we decided to use the new external icon in 12px size for links. Then we should use this new icon in link component. Also the previous external link icon was designed to be 20px size that would be too big for any external link icon. Could we add this new external 12px icon in our Codex icon list and use it in this component?

Change 850039 merged by jenkins-bot:

[design/codex@main] Link, docs: Apply design fixes

https://gerrit.wikimedia.org/r/850039

I've merged the latest patch for design fixes, meaning the Link page on the docs site now reflects these changes: https://doc.wikimedia.org/codex/main/components/mixins/link.html.

@bmartinezcalvo, can you review that page one more time and let us know if things look correct? I think that Icon changes can be handled in separate tasks and should not block the resolution of this one.

I'm not sure this ticket needs any dedicated QA (these are just links with styles, no additional behavior), so we can probably close as resolved or move to product sign-off once we have design approval.

In T316895#8353403, @egardner wrote:

@bmartinezcalvo, can you review that page one more time and let us know if things look correct? I think that Icon changes can be handled in separate tasks and should not block the resolution of this one.

Ok, I have created this other task T322014 to fix the external link icon.

@egardner what happened with the Start Icon? In some cases we could need the Start Icon in link component as specified in the spec sheet.

Is it hidden in the component or did you remove it?

In T316895#8356101, @bmartinezcalvo wrote:

@egardner what happened with the Start Icon? In some cases we could need the Start Icon in link component as specified in the spec sheet.

Is it hidden in the component or did you remove it?

Looks like that demo got removed, but I can re-introduce it to the page, in the "link with icon" section.

Change 852975 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] Link: Add example with icon at start of link

https://gerrit.wikimedia.org/r/852975

In T316895#8356101, @bmartinezcalvo wrote:

@egardner what happened with the Start Icon? In some cases we could need the Start Icon in link component as specified in the spec sheet.

Is it hidden in the component or did you remove it?

@bmartinezcalvo We two have had the conv at some place, we don't feature links with start icons in our system. That was the reason for removal.

@Volker_E I can abandon the patch I just opened if this example is not needed. In the mean time you can see a live demo here: https://852975--wikimedia-codex.netlify.app/components/mixins/link.html#link-with-icon

In T316895#8367797, @Volker_E wrote:

@bmartinezcalvo We two have had the conv at some place, we don't feature links with start icons in our system. That was the reason for removal.

@Volker_E in this comment bellow I explained that we should not use start icons in links within text paragraph.

In T316895#8346729, @bmartinezcalvo wrote:

In T316895#8346505, @Volker_E wrote:

Thanks @bmartinezcalvo!
One comment, we don't feature link icons in our environment before the link text in paragraphs.
Icons in links in running text are only used as addendum in so far is the first example in the demo misleading.

@Volker_E right, this was documented as a misuse in the Figma spec sheet and we should document it in the Codex demo too. It would be great if we can add a recommendations section with some images in our Codex demos.

But start icon can be used in standalone links so we should add the start icon as component property and explain in the demo that we will only use it when the link is not within a text block.

In T316895#8367804, @egardner wrote:

@Volker_E I can abandon the patch I just opened if this example is not needed. In the mean time you can see a live demo here: https://852975--wikimedia-codex.netlify.app/components/mixins/link.html#link-with-icon

@egardner start icon should be 20px while external link icon is 12px (as specified in the Figma spec)

If different styling is needed based on whether an Icon component appears before or after the link text, this may be difficult to achieve using CSS alone.

Say you have a link with an icon component inside it, along with link text. Your markup will look like this:

<!-- Link with icon at start -->
<a href="https://example.com"><span class="cdx-icon"><svg>...</svg></span> Link Text<a>

<!-- Link with icon at end: -->
<a href="https://example.com">Link text <span class="cdx-icon"><svg>...</svg></span><a>

So the Link contains two things: a <span> tag with the .cdx-icon class (which has an SVG inside of it), and some plain text (a TextNode, from the browser's perspective). The Icon comes either before the link text or after it.

Unfortunately, CSS has no way to distinguish between the two scenarios above (Icon before text vs Icon after text). You can write a rule that targets .cdx-icon:first-child {}, but it's going to end up applying to the Icon element regardless of whether it comes before the text or after it. CSS considers the .cdx-icon element to be both the first and last child of the <a> element; the text content is just part of the link, it doesn't get treated as a child element at all.

Firefox implements a non-standard :moz-first-node pseudo-class that would solve this problem, but we can't use it because it won't work in any other browser and is not on track to become part of the web standard.

I think the solution here is going to have to be found at the level of the Icon component, not the link styles. We should probably expose some classes or props to the user that would allow them to control the size that the icon takes inside of the link. For a CSS-only Icon component, this will probably have to be handled by certain modifier classes that apply different icons sizings from the tokens.

TLDR; I think that demonstrating the different icon sizes at start and end of link is out of scope for what we are doing here – instead this should be handled as part of future work on a CSS-only Icon component and/or as changes to how the Vue-based <CdxIcon> works.

Currently the approval of the Link LESS mixin is held up over issues with icons. We defined a style to handle icons that appear at the end of a bit of linked text, but an alternate style is needed for icons at the start of a link. We don't have a good way to differentiate between icons in different positions without adding additional markup or classes.

I'd like to propose that we remove all references to icons in links for now, and move forward with the current Link component as-is. I think that all the considerations about Icon size should be addressed at the level of the Icon component – and we may need to introduce a new CSS-only Icon style to accommodate these design requirements as well.

@bmartinezcalvo and @AnneT, what do you think about this? As a follow-up we might need to open a new task for Icon component sizes (and maybe start working on a CSS-only Icon component soon too).

In T316895#8376475, @egardner wrote:

I'd like to propose that we remove all references to icons in links for now, and move forward with the current Link component as-is. I think that all the considerations about Icon size should be addressed at the level of the Icon component – and we may need to introduce a new CSS-only Icon style to accommodate these design requirements as well.

@bmartinezcalvo and @AnneT, what do you think about this? As a follow-up we might need to open a new task for Icon component sizes (and maybe start working on a CSS-only Icon component soon too).

@egardner I've updated the Figma spec sheet removing the icon references (Start and End icons) so we can finish this MVP task now and fix the icon sizes in the future. I will create another task to add the icon and sizes in the Link component.

In T316895#8376475, @egardner wrote:

Currently the approval of the Link LESS mixin is held up over issues with icons. We defined a style to handle icons that appear at the end of a bit of linked text, but an alternate style is needed for icons at the start of a link. We don't have a good way to differentiate between icons in different positions without adding additional markup or classes.

I'd like to propose that we remove all references to icons in links for now, and move forward with the current Link component as-is. I think that all the considerations about Icon size should be addressed at the level of the Icon component – and we may need to introduce a new CSS-only Icon style to accommodate these design requirements as well.

@bmartinezcalvo and @AnneT, what do you think about this? As a follow-up we might need to open a new task for Icon component sizes (and maybe start working on a CSS-only Icon component soon too).

I agree that we should separate this work out into a new task so this one can move forward. Users can still create start and end links with the appropriate size by wrapping the link text in a <span> and setting styles on the icons via :first-child or :last-child in the meantime.

Creating a size prop in the icon component does seem like the simplest path forward, and would be useful elsewhere as well. The alternative would probably be changing Link to a full Vue component, which uses the .cdx-link() mixin internally and takes in props for startIcon and endIcon, which is what is suggested in T322631. Which path forward do we want to take?

Change 852975 abandoned by Eric Gardner:

[design/codex@main] Link: Add example with icon at start of link

Reason:

https://gerrit.wikimedia.org/r/852975

It sounds like we can live with the current version of Link as it exists here: https://doc.wikimedia.org/codex/main/components/mixins/link.html.

In the future we may need to add some additional ability to modify Icon sizes (either in CSS or in Vue) to select from certain pre-defined sizes.

For now, this "component" should be considered ready for sign-off.

Looks good to me in Chrome v106, Safari v16.0 and Firefox 106 on MacOS Monterey 12.6.

Change 859597 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v0.2.2 to v0.3.0

https://gerrit.wikimedia.org/r/859597

Change 859597 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.2.2 to v0.3.0

https://gerrit.wikimedia.org/r/859597