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

fix: add href support to actions in the Toast component #2002

Conversation

jashar11
Copy link
Contributor

@jashar11 jashar11 commented Mar 8, 2024

When passing in actions to a Toast, the href property is not acted upon if the type of the action is ToastActionType.BUTTON. With this fix, the url supplied to href is opened in a new tab when the button is clicked on.

@jashar11 jashar11 requested a review from a team as a code owner March 8, 2024 22:23
@talkor
Copy link
Member

talkor commented Mar 10, 2024

@jashar11 thanks for contributing! When using a link in the Toast it should be provided with the LINK type and not a button.
For example:

const actions = useMemo(() => [{
      type: Toast.actionTypes.LINK,
      text: "Link to action",
      href: "https://google.com"
    }], []);
    return <Toast open actions={actions}>
        General message toast
      </Toast>;
  },

See live example here.
Since it's not a fix I'm closing this PR, if there's any else fix needed please create a new issue. Thanks!

@talkor talkor closed this Mar 10, 2024
@jashar11
Copy link
Contributor Author

jashar11 commented Mar 11, 2024

I think this should still be categorised as a fix because if I simply pass

{
      type: Toast.actionTypes.BUTTON,
      content: "Button text",
      href: "https://google.com"
}

as the actions, I would expect the button click to take me to the external link. If not, then I don't see the point of having a button. At the very least, shouldn't there be an onClick property if a button is provided?

@talkor
Copy link
Member

talkor commented Mar 11, 2024

I think this should still be categorised as a fix because if I simply pass

{
      type: Toast.actionTypes.BUTTON,
      content: "Button text",
      href: "https://google.com"
}

as the actions, I would expect the button click to take me to the external link. If not, then I don't see the point of having a button. At the very least, shouldn't there be an onClick property if a button is provided?

It's a UX consideration, if you want to have a button you can use the type: Toast.actionTypes.BUTTON and a Button will be shown, if you want to have a link you can use the type: Toast.actionTypes.LINK and pass the href.

This way the behavior of the Button and Link components is preserved.
Regarding the onClick - you can pass it in the actions object along with type: Toast.actionTypes.BUTTON.

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