fix(material/button): combine MatButton and MatAnchor#30492
fix(material/button): combine MatButton and MatAnchor#30492crisbeto merged 1 commit intoangular:mainfrom
Conversation
crisbeto
requested review from
andrewseguin and
mmalerba
and removed request for
a team
crisbeto
force-pushed
the
combined-buttons
branch
5 times, most recently
from
e1f2211 to
074ed69
Compare
|
|
||
| /** Tab index for the button. */ | ||
| @Input({transform: transformTabIndex}) | ||
| tabIndex: number; |
There was a problem hiding this comment.
should this be the deprecated one instead? I think most of our components that take a tabindex use the all-lowercase spelling
There was a problem hiding this comment.
Actually we use tabIndex across a bunch of other components as well. I think it's because the native property is also called tabIndex, see https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/tabIndex
| const element: HTMLElement = this._elementRef.nativeElement; | ||
| const classList = (element as HTMLElement).classList; | ||
|
|
||
| this._isAnchor = element.tagName === 'A'; |
There was a problem hiding this comment.
probably should .toUpperCase() this to be safe
There was a problem hiding this comment.
It should be guaranteed to be uppercase. The only time tag names preserve their case is in SVG.
Currently we have two directives for each button variant: `MatButton` which applies to `button` elements and `MatButtonAnchor` which applies to anchors. This is problematic in a couple of ways: 1. The styles, which can be non-trivial, are duplicated if both classes are used. 2. Users have to think about which class they're importing. These changes combine the two classes to resolve the issues and simplify our setup. BREAKING CHANGE: `tabindex` values set as `[attr.tabindex]` set on a Material button might not work as expected. Use `tabindex` for static values, or `[tabindex]`/`[tabIndex]` for dynamic ones.
crisbeto
force-pushed
the
combined-buttons
branch
from
074ed69 to
166d46d
Compare
angular-robot
bot
added
the
detected: breaking change
crisbeto
added
target: major
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
2 participants
Currently we have two directives for each button variant:
MatButtonwhich applies tobuttonelements andMatButtonAnchorwhich applies to anchors.This is problematic in a couple of ways:
These changes combine the two classes to resolve the issues and simplify our setup.
BREAKING CHANGE:
tabindexvalues set as[attr.tabindex]set on a Material button might not work as expected. Usetabindexfor static values, or[tabindex]/[tabIndex]for dynamic ones.