Link Component #5502

Zystix · 2025-02-04T22:41:43Z

Why

Creating a new Link component.

https://cultureamp.atlassian.net/browse/KZN-2593

What

Adds the Link component, storybook tests, sticker sheets, API spec and usage guidelines.

…yles.

…order ring

changeset-bot · 2025-02-04T22:41:49Z

🦋 Changeset detected

Latest commit: 2bd25f2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@kaizen/components	Minor
@docs/storybook	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2025-02-04T22:55:10Z

✨ Here is your branch preview! ✨

Storybook

Last updated for commit 2bd25f2: Removed useReversedColors

jakubriedl · 2025-02-06T00:01:52Z

docs/pages/client-side-routing.mdx

+  return (
+    <FrontendServices {...config}>
+      {/* application code */}
+      <RacRouterProvider navigate={(href, opts) => router.push(href, undefined, opts)}>


can we make this provider part of next-services so it works for the consuming teams out of the box and without extra config?

Hey @jakubriedl did you mean before this release goes out?

I certainly think that makes sense as a future / subsequent goal, but don't think it should block this specific release since the component is agnostic to the routing solution and standard hrefs will work out of the box.

Potentially a good opportunity for @Zystix once this is done (noting we only have him till the end of Feb), but will also raise with the team to see when we can prioritize the work

yeah totally agree this shouldn't be blocker. But I'd add it there at the same time as it's just small change and write the documentation in the expects it to be there. It'll save you a lot of explaining to teams what they have to do, just say in the docs "have up-to-date next-services" and you can skip this whole section. Easier for you, and for the consumer as well

packages/components/src/Link/_docs/Link.stickersheet.stories.tsx

packages/components/src/Link/_docs/Link--api-usage-guidelines.mdx

packages/components/src/Link/_docs/Link.doc.stories.tsx

docs/pages/client-side-routing.mdx

packages/components/src/Link/_docs/Link--api-specification.mdx

packages/components/src/Link/Link.module.css

ryanseddon · 2025-02-07T00:03:59Z

packages/components/src/Link/Link.module.css

+  --text-icon-gap: var(--spacing-2);
+}
+
+[class^='small'] .isInline > *,


What's the reason for the attribute selector use here? Can it just be .small

Because of the hashing of classes it can't "see" the .small class of the component its wrapped in when inline. It does feel like a cable ties solution though

The css module will apply the correct hash as part of the postcss plugin so you can just reference it as a normal class

Didn't appear to work when we tried it? Does it look for the hash for link.module.css file?

Another note is that the .[size] .isInline > * selector is never applied due to your condition of not applying the styles when isInline is false.

This is the desired functionality, this is how we're achieving grabbing the parent Text's size class when the Link isInline. Is there anything you'd change here?

Yeah for all the selectors there you can drop the comma separated class e.g.

[class*='extra-small'] .isInline > *, .extra-small <-- don't need this

Just do

[class*='extra-small'] .isInline > * { ... }

I'll also add that the class^='small' will never actually match (at least not in storybook) as the class begins with a _ so it never finds a class starting with small

I feel like this is a bit of code smell relying on a parent component class to make changes, can we do this within code and know the size being passed down?

We can inherit the text size, the issue I think lies more in the icon sizing and spacing between the text and icon. We initially thought we might be able to leverage em to calculate a ratio based on the size but map to a cleanly between each font size.

Or were you thinking more like JS to look at the parent node and get the computed style to determine a size based off that?

packages/components/src/Link/_docs/Link.doc.stories.tsx

packages/components/src/Link/_docs/Link.stickersheet.stories.tsx

ryanseddon · 2025-02-07T03:01:30Z

packages/components/src/Link/Link.tsx

+        className={mergeClassNames(
+          styles.link,
+          isDisabled && styles.isDisabled,
+          !isInline && styles[size],


This is the condition I'm referring to, your class selector is never applied due to the fact that the size classes aren't added when the component has isInline set to true

I think this is the desired functionality, we still allow the size to be passed cos we couldn't get the discriminated union to behave. Would be good to get that sorted though

ryanseddon · 2025-02-10T01:58:00Z

Something else to consider (I don't know if both a relevant anymore) but you might want to look at seeing if adding rel="noreferrer" is something we want on any Link elements that go off to an external url.

Seems having target="_blank" now automatically does the rel="noopener" behaviour.

https://developer.chrome.com/docs/lighthouse/best-practices/external-anchors-use-rel-noopener

…omment

packages/components/src/Link/_docs/Link--api-specification.mdx

packages/components/src/Link/_docs/Link--api-usage-guidelines.mdx

mcwinter07 · 2025-02-11T00:08:59Z

packages/components/src/Link/_docs/Link.doc.stories.tsx

+
+export const LinkOpensInNewTab: Story = {
+  render: (props) => (
+    <Link {...props} target="_blank" icon={<ExternalLinkIcon role="presentation" />} />


I'd also recommend we use the end icon position with this one

mcwinter07 · 2025-02-11T00:11:55Z

packages/components/src/Link/_docs/Link.doc.stories.tsx

@@ -0,0 +1,184 @@
+import React from 'react'
+import { type Meta, type StoryObj } from '@storybook/react'
+import { ExternalLinkIcon, FlagOffIcon } from '~components/Icon'


We need to replace with these usages with the RC icon Icon component - this is why the external link icons look larger than they should be

ryanseddon · 2025-02-11T00:39:50Z

packages/components/src/Link/Link.module.css

+  --link-font-size: var(--typography-paragraph-extra-small-font-size);
+  --link-line-height: var(--typography-paragraph-extra-small-line-height);
+  --icon-font-size: 0.875rem;
+  --text-icon-gap: var(--spacing-2);


This doesn't work the way you expect it as the variable is defined within the selector range so the --text-icon-gap is always the fallback 0.5rem

Here you can see the variable is not defined in the scope of where it's used

Here it's defined (it just happens to be the same value as the fallback)

In fact all of these variables do not have any effect as they are scoped to this selector and the variables are used up the selector scope

It appears to work as intended in the stickersheet? Thats the body size

Ah, doesn't work when isInline, I'm pretty sure it used to so I might have broken something when cleaning up.

ryanseddon · 2025-02-11T00:42:35Z

packages/components/src/Link/_docs/Link.doc.stories.tsx

+export const WithText: Story = {
+  render: (props) => (
+    <>
+      <Text variant="extra-small">


Can we hook up the storybook controls to this value so we can adjust it within the story and see how it changes? At the moment the controls don't do anything

mcwinter07 · 2025-02-11T00:15:24Z

packages/components/src/Link/_docs/Link--api-specification.mdx

+
+### Icons and positioning
+
+The `icon` property abstracts the need to handle positioning and sizing of icons within the `Link` component. The `iconPosition` prop allows for the icon to be positioned to the left or right of the `children`.


Since this is a JSX element, it might be worth calling out that the intended component to be used here is the new Icon component and link out the API specification for it :)

mcwinter07 · 2025-02-11T01:18:27Z

packages/components/src/Link/Link.module.css

+  --text-icon-gap: var(--spacing-2);
+}
+
+[class^='small'] .isInline > *,


We can inherit the text size, the issue I think lies more in the icon sizing and spacing between the text and icon. We initially thought we might be able to leverage em to calculate a ratio based on the size but map to a cleanly between each font size.

Or were you thinking more like JS to look at the parent node and get the computed style to determine a size based off that?

Christian Moore added 16 commits January 14, 2025 15:34

Initial commit

c4c8ac7

label management

ed7a3f8

Use RACLink, extract LinkContent as subcomponent, add stories

56173d2

Add isInline property, clean up LinkProps type

0034e07

Underline is closer now

49b63ce

Add stickersheet

c772d51

Add pseudo states sticker sheet, abstract LinkIcon, fixed reversed st…

0560fb6

…yles.

Size styling removed from Icon, changed sizing from tshirt to typography

27365ed

Add inInline discriminated union, fix sizes in tests, fixed focused b…

ce8b70c

…order ring

Add api spec, rename props, add tests

fc4ab3b

Exracted client side routing doc

d03d39d

Add usage guidelines

56f790f

Rename prop, fix docs

4d263ab

css trial and error

e38498d

icon positioning fixed maybe

295257d

Trying to fix inline styling

b45d174

Merge branch 'main' into KZN-2593/link

7bb3c41

Zystix marked this pull request as ready for review February 4, 2025 22:52

Zystix requested a review from a team as a code owner February 4, 2025 22:52

Zystix marked this pull request as draft February 4, 2025 23:12

Christian Moore and others added 4 commits February 5, 2025 16:32

Fix font family, inline style inheritance, and icon margins

52e13dc

Fix and simplify icon sizing and positioning

aea6e52

Add changeset, usage guidelines, more css simplification

05c1410

Merge branch 'main' into KZN-2593/link

66cd9d9

Zystix marked this pull request as ready for review February 5, 2025 23:36

jakubriedl reviewed Feb 6, 2025

View reviewed changes

CSS vars use tokens as much as possible, comments removed

adbf75f

mcwinter07 reviewed Feb 6, 2025

View reviewed changes

PR Feedback

0e52fc5

ryanseddon reviewed Feb 7, 2025

View reviewed changes

Christian Moore added 2 commits February 10, 2025 08:19

Expand stickersheet

7893ed6

add line break

981ea83

Christian Moore added 3 commits February 10, 2025 14:33

Fixed reversed styles

fd66577

Add external icon to external link story, fix link in isInline code c…

4339877

…omment

use token over literal

a20d719

mcwinter07 reviewed Feb 10, 2025

View reviewed changes

Update links in doco

3046a96

mcwinter07 reviewed Feb 11, 2025

View reviewed changes

ryanseddon reviewed Feb 11, 2025

View reviewed changes

mcwinter07 reviewed Feb 11, 2025

View reviewed changes

Christian Moore added 2 commits February 11, 2025 17:38

Change isReversed to white variant, fix isInline

5fccedf

Removed useReversedColors

2bd25f2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link Component #5502

Link Component #5502

Zystix commented Feb 4, 2025

changeset-bot bot commented Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 4, 2025 •

edited

Loading

jakubriedl Feb 6, 2025

mcwinter07 Feb 6, 2025

jakubriedl Feb 6, 2025

ryanseddon Feb 7, 2025

Zystix Feb 7, 2025 •

edited

Loading

ryanseddon Feb 7, 2025

Zystix Feb 7, 2025

ryanseddon Feb 7, 2025

Zystix Feb 10, 2025

ryanseddon Feb 11, 2025

ryanseddon Feb 11, 2025 •

edited

Loading

ryanseddon Feb 11, 2025

mcwinter07 Feb 11, 2025

ryanseddon Feb 7, 2025

Zystix Feb 7, 2025

ryanseddon commented Feb 10, 2025 •

edited

Loading

mcwinter07 Feb 11, 2025

mcwinter07 Feb 11, 2025

ryanseddon Feb 11, 2025

ryanseddon Feb 11, 2025

Zystix Feb 11, 2025

Zystix Feb 11, 2025

ryanseddon Feb 11, 2025

mcwinter07 Feb 11, 2025

mcwinter07 Feb 11, 2025


		### Icons and positioning

		The `icon` property abstracts the need to handle positioning and sizing of icons within the `Link` component. The `iconPosition` prop allows for the icon to be positioned to the left or right of the `children`.

Link Component #5502

Are you sure you want to change the base?

Link Component #5502

Conversation

Zystix commented Feb 4, 2025

Why

What

changeset-bot bot commented Feb 4, 2025 • edited Loading

🦋 Changeset detected

github-actions bot commented Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zystix Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanseddon Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanseddon commented Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 4, 2025 •

edited

Loading

Zystix Feb 7, 2025 •

edited

Loading

ryanseddon Feb 11, 2025 •

edited

Loading

ryanseddon commented Feb 10, 2025 •

edited

Loading