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

Improve Button type definitions #42

Merged
merged 4 commits into from
Nov 14, 2022
Merged

Improve Button type definitions #42

merged 4 commits into from
Nov 14, 2022

Conversation

fallaciousreasoning
Copy link
Collaborator

This does the following:

  1. Makes it possible to passing through button attributes.
  2. Updates the type definitions such that the button is either a button or a link.
  3. Adds the type definitions for the Button/A tag as optional props.

It does not:

  1. Add support for events.

@petemill what do you think about this as an approach?

@fallaciousreasoning
Copy link
Collaborator Author

Hey @AlanBreck do we want to try and land this?

@AlanBreck
Copy link
Collaborator

Sounds good to me!!

@fallaciousreasoning
Copy link
Collaborator Author

All right, I've fixed the merge conflict and the formatting. Anything else need doing?

@AlanBreck
Copy link
Collaborator

Not that I can think of. The only thing that would be nice in the future is if there's any opportunity to simply for future component authors. No idea how, though. 😂

@fallaciousreasoning
Copy link
Collaborator Author

Yeah, It's definitely pretty awkward at the moment - hopefully Svelte improves this :D

@fallaciousreasoning
Copy link
Collaborator Author

@petemill what are your thoughts on this?

web-components/button/button.svelte Outdated Show resolved Hide resolved
@fallaciousreasoning fallaciousreasoning merged commit d9c7874 into main Nov 14, 2022
@fallaciousreasoning fallaciousreasoning deleted the typing-tests branch November 14, 2022 23:20
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