feat(button): add anchor button #797 #798
Conversation
|
hey @ashley-hunter as already discussed in Discord, here is my solution for that :) If you have any sugestions, please let me know |
|
@MerlinMoos - Just curious, what's the reason for the separate set of directives? Could we not just determine what the host element is in a new brnButton directive and if it's attached to an anchor do the logic you have there or maybe I'm missing something |
MerlinMoos
force-pushed
the
add-disabled-to-anchor
branch
from
697fffc to
76e1585
Compare
|
hey @thatsamsonkid you are right, I changed it. Maybe you can have a second look. |
There was a problem hiding this comment.
@MerlinMoos - Awesome thanks, I think in general this LGTM!
|
|
||
| constructor() { | ||
| const element = this._elementRef.nativeElement; | ||
| if (element.tagName === 'A') |
There was a problem hiding this comment.
This is just a nitpick/preference but I know it's technically one statement but I feel like since it spans multiple lines it's probably better to wrap it in braces
|
hey @ashley-hunter I made the changes 👍 |
MerlinMoos
force-pushed
the
add-disabled-to-anchor
branch
from
40a4ea9 to
eb62edd
Compare
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/spartan-ng/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
Primitives
Others
What is the current behavior?
Currently it's not possible to to disable a hlmBtn on a element
Closes #797
What is the new behavior?
Does this PR introduce a breaking change?
Other information