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

Allow to specify type on MatIconButton. Fixes #315 #325

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

peterblazejewicz
Copy link
Contributor

  • Material icon button changes
  • set type in Material autocomplete

Thanks!

- Material icon button changes
- set type in Material autocomplete

Thanks!
@ghost
Copy link

ghost commented Apr 3, 2020

Congratulations 🎉. DeepCode analyzed your code in 0.325 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard

/// "button", "reset", "submit"
/// </summary>
[Parameter]
public string Type { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be good to set the default to "button" ??

public string Type { get; set; } = "button";

@enkodellc
Copy link
Collaborator

enkodellc commented May 21, 2020

@peterblazejewicz Thanks for contributing. Sorry it has taken a long time to get to your PR. Please review my notes and if you wish to edit your PR then I will merge into the develop branch.

Also I recommend you update the DemoIconButton to show examples of use of the new functionality: https://github.com/SamProf/MatBlazor/blob/develop/src/MatBlazor.Demo/Demo/DemoMatIconButton.razor

@enkodellc enkodellc changed the base branch from master to develop May 21, 2020 05:41
No need to commit this file, it will be recompiled at build
@enkodellc enkodellc self-assigned this May 21, 2020
@peterblazejewicz
Copy link
Contributor Author

@enkodellc no worries, times we are living in :) I'll review the current library details, then update this PR (or just close as not relevant anymore). Anyway, I'll post here. Thanks!

@Christian-Oleson
Copy link
Contributor

@peterblazejewicz ,
There are conflicts

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.

4 participants