Skip to content

Conversation

@didoo
Copy link
Contributor

@didoo didoo commented Apr 13, 2022

📌 Summary

This is a draft PR to discuss (using real code) a possible solution to the problem of supporting LinkToExternal links in our components (these are very special kind of links, similar to the <LinkTo> but specific for Ember engines.

More context:

👉 Notice: if you want to test it yourself and play with the code:

  • clone the repo, then do a yarn install in the root of the project, and then call yarn start in the folder packages/components
  • this will build the dev version of the dummy website and run it as localhost (you will see the port in the CLI)
  • open the localhost website and navigate to the Dropdown page

🛠️ Detailed description

In this PR I have:

  • added ember-engines as both devDependency and peerDependency
  • updated the list-item/interactive to support also a LinkToExternal via a @isRouteExternal prop
  • changed one of the routes in the dropdown doc page to use an external route

🚨 Notice: this triggers an error in console, I suspect we need to configure the app to support engines

screenshot_1322

I am not sure how to proceed from here, is beyond my Ember knowledge.


👀 How to review

👉 Review by files changed

Reviewer's checklist:

  • +1 Percy if applicable

💬 Please consider using conventional comments when reviewing this PR.


@didoo didoo requested review from MelSumner and meirish April 13, 2022 17:49
@changeset-bot
Copy link

changeset-bot bot commented Apr 13, 2022

🦋 Changeset detected

Latest commit: 084a1a4

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

This PR includes changesets to release 1 package
Name Type
@hashicorp/design-system-components Patch

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

@vercel
Copy link

vercel bot commented Apr 13, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

hds-flight-website – ./

🔍 Inspect: https://vercel.com/hashicorp/hds-flight-website/9qtdXpGY1tyJyjYBWqEhqq9u1S3Y
✅ Preview: https://hds-flight-website-git-dropdown-08-add-support-866b9d-hashicorp.vercel.app

hds-components – ./

🔍 Inspect: https://vercel.com/hashicorp/hds-components/22nTAoSsJS9qt5BdqUo6Fa2zKs73
✅ Preview: https://hds-components-git-dropdown-08-add-support-for-4dc045-hashicorp.vercel.app

@didoo didoo changed the base branch from main to dropdown/06-move-toggle-files-in-sub-folder April 13, 2022 17:53
@vercel
Copy link

vercel bot commented Apr 19, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
hds-components ✅ Ready (Inspect) Visit Preview Apr 19, 2022 at 3:49PM (UTC)
hds-flight-website ✅ Ready (Inspect) Visit Preview Apr 19, 2022 at 3:49PM (UTC)

@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 19, 2022 15:49 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components April 19, 2022 15:49 Inactive
Base automatically changed from dropdown/06-move-toggle-files-in-sub-folder to dropdown-finalize-implementation April 21, 2022 14:32
@MelSumner
Copy link
Contributor

Are we sure that we need to add ember-engines as a peer dependency? This seems strange to me. Engines should be able to consume and use the addon w/o us specifically adding it, shouldn't they?

@didoo
Copy link
Contributor Author

didoo commented Apr 22, 2022

@MelSumner this is is still in draft because likely will be solved (if approved) by #216

Base automatically changed from dropdown-finalize-implementation to main April 27, 2022 15:35
Copy link
Contributor

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

Apologies for not getting to this sooner, if we need to ship this to support engines then we should ship it.

@didoo
Copy link
Contributor Author

didoo commented May 20, 2022

closing this one, since it will be fixed by #216 and #217

@didoo didoo closed this May 20, 2022
@didoo didoo deleted the dropdown/08-add-support-for-link-to-external branch June 6, 2022 12:05
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