feat: introduce `as` prop for Hyperlink #3082

adamstankiewicz · 2024-05-29T15:32:52Z

Description

Introduces as prop and types for Hyperlink to make it compatible with React Router's Link. The previously required destination prop is now only conditionally required when the as prop is a standard anchor tag (i.e., a).

If Hyperlink is used with Link, consumers would be passing the to prop, but Paragon shouldn't hardcode any router-specific props in its codebase.

Deploy Preview

https://deploy-preview-3082--paragon-openedx.netlify.app/components/hyperlink

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please request an a11y review for the PR in the #wg-paragon Open edX Slack channel.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

netlify · 2024-05-29T15:33:10Z

✅ Deploy Preview for paragon-openedx ready!

Name	Link
🔨 Latest commit	`9ead6aa`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx/deploys/6657573524ba7f00088e2b55
😎 Deploy Preview	https://deploy-preview-3082--paragon-openedx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-05-29T15:42:48Z

Codecov Report

Attention: Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.

Project coverage is 93.40%. Comparing base (c5c48aa) to head (03705d5).
Report is 3 commits behind head on release-22.x.

Files with missing lines	Patch %	Lines
src/Hyperlink/index.tsx	78.94%	4 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           release-22.x    #3082      +/-   ##
================================================
- Coverage         93.44%   93.40%   -0.05%     
================================================
  Files               251      251              
  Lines              4519     4531      +12     
  Branches           1055     1064       +9     
================================================
+ Hits               4223     4232       +9     
- Misses              293      296       +3     
  Partials              3        3

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

adamstankiewicz · 2024-05-29T16:22:58Z

src/Hyperlink/index.tsx

 import { Launch } from '../../icons';
 import Icon from '../Icon';

 export const HYPER_LINK_EXTERNAL_LINK_ALT_TEXT = 'in a new tab';
 export const HYPER_LINK_EXTERNAL_LINK_TITLE = 'Opens in a new tab';

-interface Props extends Omit<React.ComponentPropsWithRef<'a'>, 'href' | 'target'> {
+interface HyperlinkProps extends BsPrefixProps, Omit<React.ComponentPropsWithRef<'a'>, 'href' | 'target'> {
  /** specifies the URL */
  destination: string;


When using Hyperlink with Link from React Router (or GatsbyLink on the Paragon docs site example), the destination prop will not suffice as it gets passed to an href prop, leading to the link getting treated as an external URL (triggering full-page refresh).

Both Link and GatsbyLink utilize the standard hyperlink (i.e., a) when href prop is provided; to get it to work with internal routing, the to prop must be passed.

Because we use prop spreading of all attributes passed to the Hyperlink component, consumers can pass to prop but would continue to need to pass destination as it's a required prop.

We will need to determine how consumers can pass to without needing to also pass an identical value to destination (i.e., href), e.g.:

<Hyperlink as={GatsbyLink} to="/components/button" // `destination` is still a required prop even though the `to` takes precedence. destination="/components/button" > Button </Hyperlink>

@bradenmacdonald Curious if you might have any thoughts on how to mitigate this concern around to vs. href vs. destination?

Yeah, I was wondering about this.

I think one nice option is to change the Paragon hyperlink to accept to instead of destination and just treat destination as a deprecated alias for to ?

Otherwise to avoid the need to introspect as and determine what prop to pass based on some conditions, you could change the logic that currently converts destination to href and instead convert destination to both href and to, and unconditionally pass both props to the inner as component. Though I think this may display a DOM warning in the console that a doesn't accept a to attribute. So maybe some conditional logic is actually needed.

netlify · 2025-01-29T16:38:37Z

✅ Deploy Preview for paragon-openedx-v22 ready!

Name	Link
🔨 Latest commit	`03705d5`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx-v22/deploys/679a75455b83d90008862a1b
😎 Deploy Preview	https://deploy-preview-3082--paragon-openedx-v22.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

adamstankiewicz · 2025-01-29T18:22:27Z

src/Hyperlink/index.tsx

-  /** specifies the URL */
-  destination: PropTypes.string.isRequired,
+  /** specifies the component element type to render for the hyperlink */
+  // @ts-ignore


[inform] It's unclear why TS throws an error on the as prop type; as its functional, opting to @ts-ignore for now.

brian-smith-tcril

Overall this is looking great! Super happy to see this so close to landing.

I left a couple little comments with questions about onClick, any thoughts on those would be great!

brian-smith-tcril · 2025-01-30T15:43:18Z

src/Hyperlink/Hyperlink.test.tsx

    const wrapper = getByRole('link');
    expect(wrapper).toHaveAttribute('rel', 'noopener noreferrer');
  });
 });
-
-describe('event handlers are triggered correctly', () => {
-  it('should fire onClick', async () => {


It looks like onClick is still something we have an example for on the docs site https://deploy-preview-3082--paragon-openedx.netlify.app/components/hyperlink/#with-onclick

I think it might be worth keeping this test to cover the off chance someone accidentally removes onClick={onClick} from the Hyperlink component.

I removed this test case, as its behavior is technically already covered within the "renders Hyperlink" test case, i.e.:

// Clicking on the link should call the onClick handler await userEvent.click(wrapper); expect(onClick).toHaveBeenCalledTimes(1);

brian-smith-tcril · 2025-01-30T15:46:47Z

src/Menu/__snapshots__/Menu.test.jsx.snap

@@ -95,7 +95,6 @@ exports[`Menu Item renders correctly renders as expected with menu items 1`] = `
  <a
    className="pgn__hyperlink default-link standalone-link pgn__menu-item"
    href="https://en.wikipedia.org/wiki/Hyperlink"
-    onClick={[Function]}


While I think it's absolutely great that we don't have "bonus" onClicks anymore, it's not clear to me what caused this change. Any insight into this would be awesome!

The removal of the "bonus" onClick was caused by changing the defaultProps.onClick to undefined instead of the no-op function () => {}. I'm not sure having the no-op function is providing much value, so opted to remove it since its only a pass-thru prop, and isn't a breaking change.

For example, we don't do any conditional checks before calling it within Hyperlink (e.g., nothing like, onClick.?(e) implemented).

brian-smith-tcril

openedx-semantic-release-bot · 2025-01-31T13:16:40Z

🎉 This PR is included in version 22.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-semantic-release-bot · 2025-02-20T14:20:29Z

🎉 This PR is included in version 23.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

adamstankiewicz linked an issue May 29, 2024 that may be closed by this pull request

[paragon-openedx.netlify.app] Hyperlink should be compatible with React Router Link #3050

Open

adamstankiewicz marked this pull request as draft May 29, 2024 15:37

adamstankiewicz mentioned this pull request May 29, 2024

feat: add typings for <Button> #3080

Merged

10 tasks

adamstankiewicz force-pushed the ags/hyperlink-react-router-support branch from 7af05c8 to fa72a7d Compare May 29, 2024 16:22

adamstankiewicz commented May 29, 2024

View reviewed changes

adamstankiewicz force-pushed the ags/hyperlink-react-router-support branch from fa72a7d to 9ead6aa Compare May 29, 2024 16:26

adamstankiewicz changed the title ~~feat: introduce as prop for Hyperlink~~ feat: introduce as prop for Hyperlink Dec 18, 2024

adamstankiewicz force-pushed the ags/hyperlink-react-router-support branch from 9ead6aa to 765b95e Compare January 23, 2025 20:03

adamstankiewicz changed the base branch from master to release-22.x January 23, 2025 20:03

adamstankiewicz force-pushed the ags/hyperlink-react-router-support branch 3 times, most recently from 671a6e9 to 6d030f4 Compare January 29, 2025 18:21

adamstankiewicz commented Jan 29, 2025

View reviewed changes

feat: introduce as prop for Hyperlink

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

03705d5

adamstankiewicz force-pushed the ags/hyperlink-react-router-support branch from 6d030f4 to 03705d5 Compare January 29, 2025 18:36

adamstankiewicz marked this pull request as ready for review January 29, 2025 18:44

brian-smith-tcril reviewed Jan 30, 2025

View reviewed changes

brian-smith-tcril approved these changes Jan 30, 2025

View reviewed changes

adamstankiewicz merged commit c0aa4c7 into release-22.x Jan 31, 2025
9 of 10 checks passed

adamstankiewicz deleted the ags/hyperlink-react-router-support branch January 31, 2025 13:09

openedx-semantic-release-bot added the released on @22.x label Jan 31, 2025

openedx-semantic-release-bot added the released label Feb 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce `as` prop for Hyperlink #3082

feat: introduce `as` prop for Hyperlink #3082

adamstankiewicz commented May 29, 2024 •

edited

Loading

netlify bot commented May 29, 2024 •

edited

Loading

codecov bot commented May 29, 2024 •

edited

Loading

adamstankiewicz May 29, 2024 •

edited

Loading

adamstankiewicz May 29, 2024

bradenmacdonald May 29, 2024

netlify bot commented Jan 29, 2025 •

edited

Loading

adamstankiewicz Jan 29, 2025

brian-smith-tcril left a comment

brian-smith-tcril Jan 30, 2025

adamstankiewicz Jan 30, 2025

brian-smith-tcril Jan 30, 2025

adamstankiewicz Jan 30, 2025

brian-smith-tcril left a comment

openedx-semantic-release-bot commented Jan 31, 2025

openedx-semantic-release-bot commented Feb 20, 2025

feat: introduce as prop for Hyperlink #3082

feat: introduce as prop for Hyperlink #3082

Conversation

adamstankiewicz commented May 29, 2024 • edited Loading

Description

Deploy Preview

Merge Checklist

Post-merge Checklist

netlify bot commented May 29, 2024 • edited Loading

✅ Deploy Preview for paragon-openedx ready!

codecov bot commented May 29, 2024 • edited Loading

Codecov Report

adamstankiewicz May 29, 2024 • edited Loading

Choose a reason for hiding this comment

adamstankiewicz May 29, 2024

Choose a reason for hiding this comment

bradenmacdonald May 29, 2024

Choose a reason for hiding this comment

netlify bot commented Jan 29, 2025 • edited Loading

✅ Deploy Preview for paragon-openedx-v22 ready!

adamstankiewicz Jan 29, 2025

Choose a reason for hiding this comment

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril Jan 30, 2025

Choose a reason for hiding this comment

adamstankiewicz Jan 30, 2025

Choose a reason for hiding this comment

brian-smith-tcril Jan 30, 2025

Choose a reason for hiding this comment

adamstankiewicz Jan 30, 2025

Choose a reason for hiding this comment

brian-smith-tcril left a comment

Choose a reason for hiding this comment

openedx-semantic-release-bot commented Jan 31, 2025

openedx-semantic-release-bot commented Feb 20, 2025

feat: introduce `as` prop for Hyperlink #3082

feat: introduce `as` prop for Hyperlink #3082

adamstankiewicz commented May 29, 2024 •

edited

Loading

netlify bot commented May 29, 2024 •

edited

Loading

codecov bot commented May 29, 2024 •

edited

Loading

adamstankiewicz May 29, 2024 •

edited

Loading

netlify bot commented Jan 29, 2025 •

edited

Loading