Skip to content

Conversation

@didoo
Copy link
Contributor

@didoo didoo commented Apr 19, 2022

📌 Summary

Now that we have a single interactive element that handles both the normal <a> links and the <LinkTo> links, there's no reason why we should keep separate the "standalone" link in two distinct components, we can use the same method (@href vs @route) to detect which one is needed, using the underlying Hds::Interactive component that does all the work for us.

This will also simplify the adoption of the component in the consumers codebases, and its documentation.

Notice: even if under the hood it's using the Hds::Interactive component, this Hds::Link::Standalone can’t be used as a button, because there’s an assertion thrown if nor @route nor @href are passed as arguments.

🛠️ Detailed description

In this PR I have:

  • removed the LinkTo::Standalone component from the codebase
    • component
    • documentation
    • tests (percy + integration)
  • updated the Link::Standalone to use the new interactive generic component
    • in this way it serves both the old Link::Standalone and LinkTo::Standalone use cases
  • unified the documentation page - page preview
  • updated the integration tests

👀 How to review

👉 Review by files changed

Reviewer's checklist:

  • +1 Percy if applicable

💬 Please consider using conventional comments when reviewing this PR.

@changeset-bot
Copy link

changeset-bot bot commented Apr 19, 2022

⚠️ No Changeset found

Latest commit: 59d8e60

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@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 May 11, 2022 at 9:52AM (UTC)
hds-flight-website ✅ Ready (Inspect) Visit Preview May 11, 2022 at 9:52AM (UTC)

@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 19, 2022 13:49 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components April 19, 2022 13:49 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 19, 2022 13:57 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components April 19, 2022 13:57 Inactive
@didoo didoo force-pushed the spike-button-with-link-support branch from d41acff to abd2aac Compare April 20, 2022 13:29
@vercel vercel bot temporarily deployed to Preview – hds-components April 20, 2022 14:42 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 20, 2022 14:42 Inactive
@didoo didoo changed the title [WIP] Unify "Link/LinkTo::CTA" component (04) [WIP] Unify "Link/LinkTo::Standalone" component (04) Apr 20, 2022
@didoo didoo force-pushed the spike-unify-link-standalone branch from c679647 to 506ef09 Compare April 20, 2022 14:51
@vercel vercel bot temporarily deployed to Preview – hds-components April 20, 2022 14:53 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 20, 2022 14:53 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 20, 2022 15:33 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components April 20, 2022 15:33 Inactive
@didoo didoo force-pushed the spike-unify-link-standalone branch from 6adf95c to 95f9a5c Compare April 20, 2022 15:52
@vercel vercel bot temporarily deployed to Preview – hds-components April 20, 2022 15:53 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 20, 2022 15:53 Inactive
@didoo didoo requested a review from a team April 21, 2022 07:57
@didoo didoo marked this pull request as ready for review April 21, 2022 07:57
@didoo didoo changed the title [WIP] Unify "Link/LinkTo::Standalone" component (04) Unify "Link/LinkTo::Standalone" component (04) Apr 21, 2022
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 27, 2022 10:54 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components April 27, 2022 10:54 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website April 27, 2022 10:59 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components April 27, 2022 10:59 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website May 5, 2022 19:37 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components May 5, 2022 19:37 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website May 5, 2022 20:21 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components May 5, 2022 20:21 Inactive
@didoo didoo force-pushed the spike-unify-link-standalone branch from 92b39ce to 8c8e2ce Compare May 5, 2022 20:31
@didoo didoo changed the base branch from spike-button-with-link-support to spike-interactive-generic-component May 5, 2022 20:31
@vercel vercel bot temporarily deployed to Preview – hds-flight-website May 5, 2022 20:32 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components May 5, 2022 20:32 Inactive
@didoo didoo requested review from Dhaulagiri, MelSumner and alex-ju May 5, 2022 20:32
@didoo
Copy link
Contributor Author

didoo commented May 5, 2022

@alex-ju @MelSumner @Dhaulagiri can you review also this one? thanks

@vercel vercel bot temporarily deployed to Preview – hds-flight-website May 9, 2022 10:17 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components May 9, 2022 10:17 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website May 9, 2022 10:51 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-components May 9, 2022 10:51 Inactive
@alex-ju
Copy link
Member

alex-ju commented May 10, 2022

Assuming the 'Interactive' component makes its way in, I agree these two shall be merged. Couldn't spot anything off in the code.

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.

I think we should consider not removing components until we're sure of the unified interactive component approach. WDYT?

Base automatically changed from spike-interactive-generic-component to spike-BUTTON-LINK-CTA-main May 11, 2022 09:44
@didoo
Copy link
Contributor Author

didoo commented May 11, 2022

I think we should consider not removing components until we're sure of the unified interactive component approach. WDYT?

OK for me. In any case, this PR is branched from the interactive branch, and will be merged back into it, so they go hand in hand, so there's no risk of releasing one without the other. So I am merging in the main feature branch, and it will stay there until we are OK with the global changes.

@vercel vercel bot temporarily deployed to Preview – hds-components May 11, 2022 09:52 Inactive
@vercel vercel bot temporarily deployed to Preview – hds-flight-website May 11, 2022 09:52 Inactive
@didoo didoo merged commit 0d54c25 into spike-BUTTON-LINK-CTA-main May 11, 2022
@didoo didoo deleted the spike-unify-link-standalone branch May 11, 2022 10: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.

3 participants