Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Link)!: introduce v2.0 component #1890

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

booc0mtaco
Copy link
Contributor

  • add stories and styles to match new design
  • update snapshots

Test Plan:

  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Created and used an alpha publish
  • Manually tested my changes, and here are the details:

- add stories and styles to match new design
- update snapshots
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 35.00000% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 96.63%. Comparing base (d5782cd) to head (fb56ee0).
Report is 3 commits behind head on v15-components-2.0.

Files Patch % Lines
src/components/Link/Link-v2.tsx 31.57% 13 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           v15-components-2.0    #1890      +/-   ##
======================================================
- Coverage               97.16%   96.63%   -0.53%     
======================================================
  Files                     106      107       +1     
  Lines                    2330     2350      +20     
  Branches                  546      556      +10     
======================================================
+ Hits                     2264     2271       +7     
- Misses                     64       77      +13     
  Partials                    2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@booc0mtaco booc0mtaco changed the title feat(Link)!: add v2 component feat(Link)!: introduce v2.0 component Mar 13, 2024
*
* **Default is `"a"`**.
*/
as: string | React.ElementType;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this to support links link in Remix. Will likely need some work to support fully, but will be able to test once i get thru the first wave of component implementations

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Challenge I see: Remix links currently use to instead of href

So to enable passing in a Remix link to here, we'd probably need to figure out how to support passing the EDS Link's href prop to the Remix Link's to somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to replicate what we do using ClickableStyle and also get rid of that component in the next version :).

I didn't want to copy how ClickableStyle works in here, b/c it seems to lose all the typing information and converts everything to any :( What does make sense over there is that it uses a type param to combine the types of Remix's Link with the component props.

I'm guessing this was part of why you needed to export the types earlier?

Copy link
Contributor

@ahuth ahuth Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on that goal, and not wanting to do ClickableStyle.

My point is that to achieve:

<EdsLink
  as={RemixLink}
  href={...} // or maybe to={...}?
/>

we'll need to figure out how to handle href vs to.

Copy link
Contributor

@ahuth ahuth Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to do that would be for the Eds Link to take href and always pass both it as both href and to on to the as component...

Kind of annoying, but that's the simplest thing I can see (at least until Remix updates its Link to use the standard href).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on that goal, and not wanting to do ClickableStyle.

My point is that to achieve:

<EdsLink
  as={RemixLink}
  href={...} // or maybe to={...}?
/>

we'll need to figure out how to handle href vs to.

Yah, I think you misunderstood. The way ClickableStyle supports to/href is weird, but "works". So we are saying the same thing basically.

I want to avoid polluting EDS with anything specific to Remix or some other framework, as well. I like the TypeParam approach (e.g., combine EDSLink with Framework type, with option or to Omit). Once I get thru the re-theming, I'll try a few things.

@booc0mtaco booc0mtaco requested a review from a team March 13, 2024 19:40
* Component for making styled anchor tags. Links allow users to navigate within or between a web page(s) or app(s).
*
*/
export const Link = forwardRef<HTMLAnchorElement, LinkProps>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't wait until React removes forwardRef in React 19

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, what will happen instead? I never read the upcoming docs for things besides HTML/CSS/JS :P


type LinkHTMLElementProps = Omit<
React.AnchorHTMLAttributes<HTMLAnchorElement>,
'disabled'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's omitting disabled all about? I thought disabled already wasn't an attribute on anchor elements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this is probably a carry-over from the ClickableStyles stuff that wasn't removed a while back.

@booc0mtaco
Copy link
Contributor Author

Will merge this in to the v15 branch, and come back to the handling of router links toward the end of the month. Marking a few things in code so I don't forget first

- also add in the nested V2 folder in storybook, and export
@booc0mtaco booc0mtaco merged commit bdce10a into v15-components-2.0 Mar 19, 2024
8 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EDS-1142 branch March 19, 2024 20:59
@booc0mtaco booc0mtaco mentioned this pull request Jun 20, 2024
booc0mtaco added a commit that referenced this pull request Jun 20, 2024
## [15.0.0](v14.0.0...v15.0.0) (2024-06-20)


### ⚠ BREAKING CHANGES

* **TextareaField:** introduce 2.0 component (#1911)
* **Card:** introduce 2.0 component (#1908)
* **Modal:** introduce 2.0 component (#1907)
* **Toast:** introduce 2.0 component (#1906)
* **Tooltip:** introduce 2.0 component (#1905)
* **BannerNotification:** introduce 2.0 component (#1904)
* **InlineNotification:** introduce 2.0 component (#1903)
* **Accordion:** introduce 2.0 component (#1901)
* **Select:** introduce 2.0 component (#1899)
* **config:** use literal values in style-dictionary config (#1900)
* **InputField:** introduce 2.0 component (#1898)
* **Checkbox:** introduce 2.0 component (#1897)
* **Radio:** introduce 2.0 component (#1895)
* **Menu:** introduce 2.0 component (#1894)
* **TabGroup:** introduce 2.0 component (#1892)
* **NumberIcon:** introduce 2.0 component (#1891)
* **Link:** introduce v2.0 component (#1890)
* **Button:** introduce v2.0 component (#1889)
* **tokens:** remove units from design system tokens (#1912)
* **Icon:** introduce 2.0 component (#1925)
* adjust responsive and spacing tokens/config (#1933)
* **AppNotification:** introduce 2.0 component (#1945)
* **ToastNotification:** rename from Toast to ToastNotification (#1944)
* **typography:** update typography tokens and fonts (#1942)
* **FieldLabel:** introduce 2.0 component (#1953)
* **LoadingIndicator:** introduce 2.0 component (#1963)
* update tier 1 and 2 typography tokens (#1969)
* move from isWarning and isError to status prop (#1973)
* pre-cleanup changes to prepare for v15 (#1992)

### Features

* **Accordion:** introduce 2.0 component ([#1901](#1901)) ([cf2086b](cf2086b))
* add new v2.0 component tokens ([#1888](#1888)) ([3607a5d](3607a5d))
* **BannerNotification:** introduce 2.0 component ([#1904](#1904)) ([cda9e4b](cda9e4b))
* **Button:** introduce v2.0 component ([#1889](#1889)) ([a6b446f](a6b446f))
* **Card:** introduce 2.0 component ([#1908](#1908)) ([cd21b49](cd21b49))
* **Checkbox:** introduce 2.0 component ([#1897](#1897)) ([f3fc767](f3fc767))
* **config:** use literal values in style-dictionary config ([#1900](#1900)) ([e470f4b](e470f4b))
* **InlineNotification:** introduce 2.0 component ([#1903](#1903)) ([7bff52d](7bff52d))
* **InputField:** introduce 2.0 component ([#1898](#1898)) ([a3d3984](a3d3984))
* **Link:** introduce v2.0 component ([#1890](#1890)) ([bdce10a](bdce10a))
* **Menu:** introduce 2.0 component ([#1894](#1894)) ([3f540f9](3f540f9))
* **Modal:** introduce 2.0 component ([#1907](#1907)) ([d14c963](d14c963))
* **NumberIcon:** introduce 2.0 component ([#1891](#1891)) ([cc7e140](cc7e140))
* **Radio:** introduce 2.0 component ([#1895](#1895)) ([8ef797f](8ef797f))
* **Select:** introduce 2.0 component ([#1899](#1899)) ([1164b90](1164b90))
* **TabGroup:** introduce 2.0 component ([#1892](#1892)) ([e952d33](e952d33))
* **TextareaField:** introduce 2.0 component ([#1911](#1911)) ([a68a255](a68a255))
* **Toast:** introduce 2.0 component ([#1906](#1906)) ([8bce819](8bce819))
* **Tooltip:** introduce 2.0 component ([#1905](#1905)) ([ded98b2](ded98b2))
* add support for extended nav and action components ([#1918](#1918)) ([f4a541e](f4a541e))
* **tokens:** remove units from design system tokens ([#1912](#1912)) ([ab666cf](ab666cf))
* **Icon:** introduce 2.0 component ([#1925](#1925)) ([3e40638](3e40638))
* adjust responsive and spacing tokens/config ([#1933](#1933)) ([b19e453](b19e453))
* **AppNotification:** introduce 2.0 component ([#1945](#1945)) ([489e8d9](489e8d9))
* **ToastNotification:** rename from Toast to ToastNotification ([#1944](#1944)) ([07284c2](07284c2))
* **typography:** update typography tokens and fonts ([#1942](#1942)) ([3140996](3140996))
* **Link:** allow nodes to be used in link body ([#1950](#1950)) ([01970a2](01970a2))
* allow react nodes on subtitle ([#1954](#1954)) ([fc6bb78](fc6bb78))
* **FieldLabel:** introduce 2.0 component ([#1953](#1953)) ([6198b9e](6198b9e))
* add eds-migrate command ([b45061e](b45061e))
* **tokens:** add border-utility-inteactive-secondary tokens ([#1959](#1959)) ([72daa0b](72daa0b))
* add `eds-migrate` script for running codemods on major version upgrades ([#1951](#1951)) ([109a0e5](109a0e5))
* **LoadingIndicator:** introduce 2.0 component ([#1963](#1963)) ([26faab7](26faab7))
* **LoadingIndicator:** update stroke width in Button ([#1964](#1964)) ([bac3594](bac3594))
* **Menu:** set new default for menu trigger ([#1965](#1965)) ([24815c2](24815c2))
* **Select:** support horizontal labels ([#1962](#1962)) ([675ad5f](675ad5f))
* **TabGroup:** add inverse variant treatment ([#1960](#1960)) ([402e433](402e433))
* **tokens:** add border-utility-inteactive-secondary tokens ([#1959](#1959)) ([72daa0b](72daa0b))
* **Button:** update interactive styles for tertiary buttons ([#1970](#1970)) ([288da8c](288da8c))
* update tier 1 and 2 typography tokens ([#1969](#1969)) ([9dea463](9dea463))
* **Link:** add inverse variant (with variant prop) ([#1972](#1972)) ([bdbf9df](bdbf9df))
* move from isWarning and isError to status prop ([#1973](#1973)) ([56066ae](56066ae))
* **Icon:** add new and updated icons ([#1981](#1981)) ([fc7f842](fc7f842))
* **Link:** add inverse text visited color token and apply ([#1982](#1982)) ([98dec99](98dec99))
* **ToastNotification:** add dismissType with automated dismissing ([#1980](#1980)) ([8545f14](8545f14))
* **Tooltip:** add inverse variant ([#1984](#1984)) ([d6ccc8d](d6ccc8d))
* **Typography:** add typography-overline-lg ([#1993](#1993)) ([36eb3c9](36eb3c9))
* pre-cleanup changes to prepare for v15 ([#1992](#1992)) ([dbce0d2](dbce0d2))


### Bug Fixes

* disambiguate old token colors ([#1913](#1913)) ([758eb2d](758eb2d))
* **Button:** allow as prop on button ([#1994](#1994)) ([37d1f5b](37d1f5b))
* **FieldNote:** fix icon alignment and size issues ([367717e](367717e))
* **Button:** align disabled treatment to latest design ([#1931](#1931)) ([01a9d71](01a9d71))
* sync color tokens with design ([#1929](#1929)) ([146df70](146df70))
* **Button:** address QA design feedback ([#1940](#1940)) ([1e431b4](1e431b4))
* **ButtonGroup:** address QA design updates ([#1943](#1943)) ([1595a18](1595a18))
* **InlineNotification:** address QA updates ([#1937](#1937)) ([d96ab79](d96ab79))
* **TabGroup:** address QA comments and designs ([#1938](#1938)) ([48e92d4](48e92d4))
* **TextareaField:** address QA updates ([#1936](#1936)) ([3aa5c94](3aa5c94))
* update token and theming ([#1946](#1946)) ([5546205](5546205))
* **Accordion:** address QA updates from design ([#1948](#1948)) ([56fa437](56fa437))
* **AppNotification:** apply usage feedback ([#1949](#1949)) ([06cc194](06cc194))
* **NumberIcon:** address QA design notes ([#1947](#1947)) ([007d757](007d757))
* **Card:** address QA design and eng feedback ([#1958](#1958)) ([14c73fe](14c73fe))
* **Modal:** address QA updates ([#1957](#1957)) ([0d5a414](0d5a414))
* handle more cases ([a7e5830](a7e5830))
* **FieldNote:** adjust layout for icon and text lockup ([5788fe5](5788fe5))
* **Button:** add in missing inverse disabled treatments ([#1976](#1976)) ([8389f35](8389f35))
* **Button:** mark disable usage as invalid ([#1977](#1977)) ([99a70d3](99a70d3))
* **Button:** use text-utility-default state tokens ([#1979](#1979)) ([e754588](e754588))
* **Link:** address problems with link color inheritance ([#1975](#1975)) ([78d173e](78d173e))
* **Link:** align icons in flexed standalone container ([#1978](#1978)) ([3a6a20d](3a6a20d))
* **Button:** align button icons and text with layouts ([#1988](#1988)) ([33fbd51](33fbd51))
* remove circular dependency for radio/checkbox ([dffed3c](dffed3c))
* use aria-disabled when disabled prop is applied ([#1987](#1987)) ([1fe3b0f](1fe3b0f))
* handle required disabled field hints ([#1990](#1990)) ([11c0883](11c0883))
* **Select:** allow attachment of headlessUI props to label ([#1991](#1991)) ([c3ce70f](c3ce70f))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants