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

Commit

Permalink
fix(nav-bar): improve focus behavior for click events (#11600)
Browse files Browse the repository at this point in the history
<!-- 
Filling out this template is required! Do not delete it when submitting a Pull Request! Without this information, your Pull Request may be auto-closed.
-->
## PR Checklist
Please check that your PR fulfills the following requirements:
- [x] The commit message follows [our guidelines](https://github.com/angular/material/blob/master/.github/CONTRIBUTING.md#-commit-message-format)
- [x] Tests for the changes have been added or this is not a bug fix / enhancement
- [x] Docs have been added, updated, or were not required

## PR Type
What kind of change does this PR introduce?
<!-- Please check the one that applies to this PR using "x". -->
```
[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:
```

## What is the current behavior?
Clicking on nav items with a mouse causes the focus to move to the previously clicked nav item instead of the newly clicked nav item.
<!-- Please describe the current behavior that you are modifying and link to one or more relevant issues. -->
Issue Number: 
Fixes #11591. Relates to #11494. Closes #11598.

## What is the new behavior?
- Focus is moved to the new nav item after it is clicked.
- remove hover style that is inconsistent with spec
- add test from @codymikol 

## Does this PR introduce a breaking change?
```
[ ] Yes
[x] No
```
<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->
<!-- Note that breaking changes are highly unlikely to get merged to master unless the validation is clear and the use case is critical. -->

## Other information
Thank you to @codymikol for doing the initial investigation into this regression!
  • Loading branch information
Splaktar authored and mmalerba committed Feb 1, 2019
1 parent a191a8e commit e64875d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 9 deletions.
16 changes: 12 additions & 4 deletions src/components/navBar/navBar.js
Expand Up @@ -114,10 +114,9 @@ function MdNavBar($mdAria, $mdTheming) {

/**
* Controller for the nav-bar component.
*
* TODO update this with a link to tablist when that implementation gets merged.
* Accessibility functionality is implemented as a site navigator with a listbox, according to
* https://www.w3.org/TR/wai-aria-practices/#Site_Navigator_Tabbed_Style.
* Accessibility functionality is implemented as a tablist
* (https://www.w3.org/TR/wai-aria-1.0/complete#tablist) and
* tabs (https://www.w3.org/TR/wai-aria-1.0/complete#tab).
*
* @param {!angular.JQLite} $element
* @param {!angular.Scope} $scope
Expand Down Expand Up @@ -196,6 +195,7 @@ MdNavBarController.prototype._initTabs = function() {
MdNavBarController.prototype._updateTabs = function(newValue, oldValue) {
var self = this;
var tabs = this._getTabs();
var sameTab = newValue === oldValue;

// this._getTabs can return null if nav-bar has not yet been initialized
if (!tabs) return;
Expand All @@ -217,6 +217,11 @@ MdNavBarController.prototype._updateTabs = function(newValue, oldValue) {

this._$timeout(function() {
self._updateInkBarStyles(newTab, newIndex, oldIndex);
// Don't change focus when there is no newTab, the new and old tabs are the same, or when
// called from MdNavBarController._initTabs() which would have no oldTab defined.
if (newTab && oldTab && !sameTab) {
self._moveFocus(oldTab, newTab);
}
});
};

Expand Down Expand Up @@ -596,6 +601,9 @@ function MdNavItem($mdAria, $$rAF, $mdUtil, $window) {
});

navButton.on('click', function() {
// This triggers a watcher on mdNavBar.mdSelectedNavItem which calls
// MdNavBarController._updateTabs() after a $timeout. That function calls
// MdNavItemController.setSelected() for the old tab with false and the new tab with true.
mdNavBar.mdSelectedNavItem = mdNavItem.name;
scope.$apply();
});
Expand Down
5 changes: 0 additions & 5 deletions src/components/navBar/navBar.scss
Expand Up @@ -34,11 +34,6 @@ $md-nav-bar-height: 48px;
&:focus {
outline: none;
}

&:hover {
background-color: inherit;
}

}

md-nav-ink-bar {
Expand Down
11 changes: 11 additions & 0 deletions src/components/navBar/navBar.spec.js
Expand Up @@ -183,6 +183,17 @@ describe('mdNavBar', function() {
expect($scope.selectedTabRoute).toBe('tab2');
});

it('should add the md-focused class when focused', function () {
$scope.selectedTabRoute = 'tab1';
createTabs();
var tab2Ctrl = getTabCtrl('tab2');
angular.element(tab2Ctrl.getButtonEl()).triggerHandler('focus');
angular.element(tab2Ctrl.getButtonEl()).triggerHandler('click');
$scope.$apply();
$timeout.flush();
expect(tab2Ctrl.getButtonEl().classList.contains('md-focused')).toBe(true);
});

it('adds ui-sref-opts attribute to nav item when sref-opts attribute is ' +
'defined', function() {
create(
Expand Down

0 comments on commit e64875d

Please sign in to comment.