Extend the "Button" component to support also links as actions (02) #220
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). hds-flight-website – ./🔍 Inspect: https://vercel.com/hashicorp/hds-flight-website/2qfmBKXqyBrLGJtApKgUE1z9zJ27 hds-components – ./🔍 Inspect: https://vercel.com/hashicorp/hds-components/ByKn5di8B7B9F9BY4VKatEEWwtSi |
|
didoo
changed the title
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…L attribute is used instead of the `@href` Ember argument
didoo
force-pushed
the
spike-button-with-link-support
branch
from
d41acff to
abd2aac
Compare
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
I've removed the @type argument from the API in favour of the type native attribute (this simplifies the code)
Notice: by default type="button" (see Hds::Interactive code)
|
|
||
| // SPECIAL | ||
|
|
||
| // we apply a visual treatment to alert the developer if a "href" HTML attribute is used (instead of the "@href" Ember argument) |
There was a problem hiding this comment.
please let me know what you think of this
There was a problem hiding this comment.
seems like a good idea; if we end-up needing this in other places there may be value in converting it into a mixin
There was a problem hiding this comment.
Reiterating in writing that we need to plan future work to improve this, as it's not an ideal outcome to have a component that renders something different to the browser than the name suggests. We have some constraints here and need to do some additional planning, but want to iterate on this for an improved outcome.
|
|
||
| // SPECIAL | ||
|
|
||
| // we apply a visual treatment to alert the developer if a "href" HTML attribute is used (instead of the "@href" Ember argument) |
There was a problem hiding this comment.
seems like a good idea; if we end-up needing this in other places there may be value in converting it into a mixin
Remove the "Link/LinkTo::CTA" component (03)
3 participants
📌 Summary
This is an attempt to solve once for all the issue of "links that look like buttons" in our products.
The previous attempt using a
Link::CTAcomponent (see #115) was followed by other issues raised by the developers, this time around the need of asecondarystyling for the link (see this Slack thread, but also this Slack thread, this other Slack thread, this Slack thread too, this comment, this Jira ticket, this GitHub PR).Adding a secondary style for the
Link::CTA, which by its own definition is intended to be a design element that [...] stands out from the rest of the page[and] tells the user “this is the most important action on this page” doesn't make sense at this point. The only pragmatic solution is to modify theButtoncomponent and allow it to handle links too.This change will facilitate the migration of the Cloud UI codebase to HDS buttons, unblocking some of the tasks that were in standby, waiting for alternatives to the existing UI implementations based on Structure components.
🛠️ Detailed description
In this PR I have:
buttonto use the newinteractivegeneric componentspacekey event handler when the element is alink(see Enable interactive links to be actioned using the space key #291)@typeargument from the API in favour of thetypenative attribute (this simplified the code)buttonto work differently between buttons and linkshrefHTML attribute is used instead of the@hrefEmber argument🤔 Things to discuss/decide
hrefinstead of a@href🔗 Links
👀 How to review
👉 Review by files changed
Reviewer's checklist:
💬 Please consider using conventional comments when reviewing this PR.