Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

feat/bug: md-button mdNoFocusStyle not working for md-list-item / md-card #8691

Closed
graphefruit opened this issue Jun 7, 2016 · 2 comments
Closed
Assignees
Labels
has: Pull Request A PR has been created to address this issue type: enhancement

Comments

@graphefruit
Copy link

graphefruit commented Jun 7, 2016

Actual behavior:

  • What is the issue? * The mdNoFocusStyle can be attach on normal md-buttons but cant be attached on md-list-items aswell as md-cards with an ng-click
    The md-list-item aswell as the md-card generate an internal button without the mdNoFocusStyle
  • What is the expected behavior? The mdNoFocusStyle should be attached on md-list-items / md-cards

CodePen or Steps to reproduce the issue: *
https://material.angularjs.org/latest/demo/list

Thoughts:
After this would be a bigger task, the question is, if it would be possible/better to add the posibility to disable the mdNoFocusStyle globally for all buttons.

I wrote a simple code for this:

if (!angular.isDefined(attr.mdNoFocusStyle) && !$injector.has('$MD_DISABLE_FOCUS_STYLE')) {
  // restrict focus styles to the keyboard
  scope.mouseActive = false;
  element.on('mousedown', function() {
    scope.mouseActive = true;
    $timeout(function(){
      scope.mouseActive = false;
    }, 100);
  })
  .on('focus', function() {
    if (scope.mouseActive === false) {
      element.addClass('md-focused');
    }
  })
  .on('blur', function(ev) {
    element.removeClass('md-focused');
  });
}

in .config -> $provide.constant('$MD_DISABLE_FOCUS_STYLE', '');

Also this would enable a performance improvement for Internet Explorer (specially on mobile versions aswell).
The problem is:

  1. $timeout - Triggers a $q.promise chain for angular. (maybe here use $timeout(func,timeout,false))
  2. The addClass/removeClass triggers a complete repaint of the page.
  3. Because of the md-list-item/md-card: Many event-listeners cant be disabled (specially on ng-repeat scenarios)

Angular Versions: *

  • Angular Version: 1.5.5
  • Angular Material Version: RC-5

Additional Information:

  • Browser Type: All browsers
  • Browser Version: All versions
  • OS: All OS
  • Stack Traces:
@devversion
Copy link
Member

devversion commented Jun 11, 2016

The approach with the $MD_DISABLE_FOCUS_STYLE constant is not something we should support.

Once we introduce a constant, like this, the API will get really confusing and users expect the same constants for other components as well.

The focus style can be easily overwritten per CSS - I know you talked about the promise chain, but this is IMO not a big deal, because otherwise we will have to introduce this to other components with $q promises as well (as said above)


Indeed, we should probably support the md-no-focus-style attribute on the md-list-item.

@graphefruit What is about the card component - Can you explain it a bit?

@devversion devversion self-assigned this Jun 11, 2016
devversion added a commit to devversion/material that referenced this issue Jun 11, 2016
* Adds support for `md-no-focus-style` attribute forwarding
* Add documentation for `md-menu` as proxy item
* Enhance tests for `md-menu` inside of `md-list-item`

Closes angular#8691.
@devversion devversion added has: Pull Request A PR has been created to address this issue type: enhancement labels Jun 11, 2016
@ThomasBurleson ThomasBurleson added this to the - Backlog milestone Jun 12, 2016
@graphefruit
Copy link
Author

@devversion sorry for late response, got no PC.
Card-Component: I had a wrong look and thought it would be a card, but was a list-item (just ignore please).

Thanks so far & have a great week

devversion added a commit to devversion/material that referenced this issue Jun 13, 2016
* Refactors `md-no-focus-style` attribute into the `md-no-focus` class.
* Adds support for `md-no-focus` class forwarding for the list item.
* Add documentation for `md-menu` as proxy item
* Enhance tests for `md-menu` inside of `md-list-item`

BREAKING CHANGE: `md-no-focus-style` attribute on `md-button` is now a
class (`.md-no-focus`)

Closes angular#8691.
devversion added a commit to devversion/material that referenced this issue Jun 13, 2016
* Refactors `md-no-focus-style` attribute into the `md-no-focus` class.
* Adds support for `md-no-focus` class forwarding for the list item.
* Add documentation for `md-menu` as proxy item
* Enhance tests for `md-menu` inside of `md-list-item`

BREAKING CHANGE: `md-no-focus-style` attribute on `md-button` is now a
class (`.md-no-focus`)

Closes angular#8691.
devversion added a commit to devversion/material that referenced this issue Jun 13, 2016
* Refactors `md-no-focus-style` attribute into the `md-no-focus` class.
* Adds support for `md-no-focus` class forwarding for the list item.
* Add documentation for `md-menu` as proxy item
* Enhance tests for `md-menu` inside of `md-list-item`

BREAKING CHANGE: `md-no-focus-style` attribute on `md-button` is now a
class (`.md-no-focus`)

Closes angular#8691.
@Splaktar Splaktar removed this from the - Backlog milestone Feb 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has: Pull Request A PR has been created to address this issue type: enhancement
Projects
None yet
Development

No branches or pull requests

4 participants