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

WC vs. WP external <Link>s #984

Open
tomalec opened this issue Aug 28, 2021 · 4 comments
Open

WC vs. WP external <Link>s #984

tomalec opened this issue Aug 28, 2021 · 4 comments
Labels
needs design The issue requires design input/work from a designer. type: technical debt - external devX This issue/PR suffers from the ergonomics of external tools/dependencies

Comments

@tomalec
Copy link
Member

tomalec commented Aug 28, 2021

Describe the bug:

I noticed that for external documentation links we use @woocommerce/components.Link with external prop, and occasionally add <ExternalIcon>, then manually tweak it, to look a tad better.

I wonder:

  1. Why don't we use @wordpress/components.ExternalLink?
  2. Why WC one does not attach the icon as the WP one?
    1. That question/or feature request could be propagated to wc-admin if we don't know.
  3. Was that a conscious decision to make <ExternalIcon> non-clickable in unsupported countries notice?

image
WordPress <ExternalLink> on left, WooCommerce > GLA <AppDocumentationLink /><ExternalIcon /> to the right.

Personally, I find the left one more appealing, plus it reduces the style tweaks on our end and just gets it for free from WP. Also, (however without deeper review of all cases), as a user, I, like it when the UI constantly visually marks all external links, so I know, when to expect to be navigated away.

@j111q, @eason9487 Do you have any comments on that?

Additional notes:

  1. Context: asking as I'm about to add another external link in notice for Speak the same currency #363 (comment)
  2. Figma link for reference https://www.figma.com/file/jZUpa8eTrnrK1Lwt2ry7zk/Google-Listings-%26-Ads-v1.0?node-id=3058%3A573
@tomalec tomalec added type: question The issue is a question about how code works. needs design The issue requires design input/work from a designer. labels Aug 28, 2021
@eason9487
Copy link
Member

eason9487 commented Aug 30, 2021

TIL: there's a @wordpress/components.ExternalLink component. 👍 Looks like it's suitable than composing them on our own if wrapping the <ExternalIcon> in the clickable link is fine.

Also, (however without deeper review of all cases), as a user, I, like it when the UI constantly visually marks all external links, so I know, when to expect to be navigated away.

👍 upvote for this.

@j111q
Copy link

j111q commented Aug 30, 2021

I don't think it was a conscious choice, your suggestion sounds good! 👍 Thanks for checking 😄

@jconroy
Copy link
Member

jconroy commented Aug 30, 2021

@ecgan probably has more background but I think there might be have been a gotcha or some preference to do with tracks events

@ecgan
Copy link
Member

ecgan commented Aug 30, 2021

TIL for me too, didn't notice the @wordpress/components.ExternalLink component. 👍

  1. Why don't we use @wordpress/components.ExternalLink?

I think that within us WooCommerce ecosystem, we should use the @woocommerce/components's Link component. Currently, inside the component, there isn't really any code specific for type="external". However, just in case in the future there are component code change for it, our plugin would be ready for it (we just need to update the component package, hopefully).

Why WC one does not attach the icon as the WP one?

  1. That question/or feature request could be propagated to wc-admin if we don't know.

It has always been that way, as far as I know. Maybe that was how WooCommerce wanted it.

We can probably try to implement the icon in @woocommerce/components's Link component in WC Admin repo. If that's not possible, we can do that within our own repo.

3. Was that a conscious decision to make <ExternalIcon> non-clickable in unsupported countries notice?

Yeah I agree that the left one is better. The text and icon has the same color, and the click target is bigger.

as a user, I, like it when the UI constantly visually marks all external links, so I know, when to expect to be navigated away.

I agree too. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs design The issue requires design input/work from a designer. type: technical debt - external devX This issue/PR suffers from the ergonomics of external tools/dependencies
Projects
None yet
Development

No branches or pull requests

5 participants