-
Notifications
You must be signed in to change notification settings - Fork 6.8k
bug(focus indicators): Focus indicators are now positioned. #19345
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but @jelbourn might have an opinion on this.
@jelbourn Before merging this, let's close on whether the style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replied via email, but I'm good with adding the relative
style in mat-core
(assuming the TGP works out)
|
||
// The focus indicator should match the bounds of the entire button. | ||
.mat-mdc-focus-indicator { | ||
@include mat-fill(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this change, this element had essentially no styles, and thus the focus indicator actually rendered relative to the parent button. Now that we've added position: relative
to this element, the fact that it has no styles is problematic (the focus indicator doesn't render with the correct bounding box). Consequently, use mat-fill
to make this element match the bounds of the entire button.
TGP LGTM, sent the link to you @jelbourn. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just minor naming comment
quick fix date picker focus indicator bug.
Addressed naming comment, rebased and squashed commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…lesheets are ordered improperly in Karma unit tests.
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. |
.mat-ripple
is positioned in order to create an easy API for developers, every.mat-focus-indicator
should be positioned. This way, when developers add the class.mat-focus-indicator
to their own custom focusable elements they do not also need to position the element..mat-focus-indicator
, a few components that setposition: absolute
on the associated element need to have their specificity bumped.