Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

feat(interaction): added service to detect last interaction #5589

Conversation

devversion
Copy link
Member

Sidenav want's to restore focus after the pane gets closed, this should only happen for keyboard interactions.
The service got created to use that feature for other components too

Fixes #5563 Fixes #5434 Closes #5583

@devversion
Copy link
Member Author

I added a few more tests, more test will come as soon as possible
@EladBezalel Can you please review my PR

@@ -227,7 +227,7 @@ function SidenavDirective($mdMedia, $mdUtil, $mdConstant, $mdTheming, $animate,
*/
function postLink(scope, element, attr, sidenavCtrl) {
var lastParentOverFlow;
var triggeringElement = null;
var triggeringElement = null, triggeringInteractionType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please separate to another declaration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed...the trickiness here hinders code readability.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't happen again, didn't know about this style policy ;)

@EladBezalel
Copy link
Member

Please fix and squash your commits to 1 commit

@devversion
Copy link
Member Author

Commits successfully squashed, thanks for reviewing @EladBezalel @marcysutton

@devversion devversion force-pushed the fix/sidenav-focus-restore-service branch from 7f45663 to 5dd246d Compare November 9, 2015 16:38
devversion added a commit to devversion/material that referenced this pull request Nov 9, 2015
Sidenav want's to restore focus after the pane gets closed, this should only happen for keyboard interactions.
The service got created to use that feature for other components too

Fixes angular#5563 Fixes angular#5434 Closes angular#5583 Closes angular#5589

feat(interaction): add test for interaction which fakes a keyboard input and validates it

test(interaction): add interaction spec for mousedown event + formats 4 spaces to 2

style(interaction): fix styling of the previous commits
@@ -0,0 +1,26 @@
describe("$mdInteraction", function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to see some additional tests here. Since the focus style hinges off of .md-focused being added, we could test that it gets added on keyboard interaction and not mouse.

@devversion devversion force-pushed the fix/sidenav-focus-restore-service branch 3 times, most recently from febc90e to 29d4146 Compare November 11, 2015 20:43
devversion added a commit to devversion/material that referenced this pull request Nov 11, 2015
Sidenav want's to restore focus after the pane gets closed, this should only happen for keyboard interactions.
The service got created to use that feature for other components too

Fixes angular#5563 Fixes angular#5434 Closes angular#5583 Closes angular#5589

feat(interaction): add test for interaction which fakes a keyboard input and validates it

test(interaction): add interaction spec for mousedown event + formats 4 spaces to 2

style(interaction): fix styling of the previous commits

test(sidenav): add sidenav focus restore test triggered by keyboard

test(sidenav): add focus restore test for mouse and patch other one

fix(sidenav): test
@devversion
Copy link
Member Author

Added two tests into the sidenav.specs and squashed commits too
@EladBezalel @marcysutton

var el;
inject(function($compile, $rootScope) {
var parent = angular.element(document.body);
el = angular.element('<button aria-label="Toggle Menu">Toggle</button>');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that the tests show correct requirements, you don't need aria-label here since the button has text in it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@devversion devversion force-pushed the fix/sidenav-focus-restore-service branch from 63e270c to 0cf58e4 Compare November 12, 2015 13:39
devversion added a commit to devversion/material that referenced this pull request Nov 12, 2015
Sidenav want's to restore focus after the pane gets closed, this should only happen for keyboard interactions.
The service got created to use that feature for other components too

Fixes angular#5563 Fixes angular#5434 Closes angular#5583 Closes angular#5589

feat(interaction): add test for interaction which fakes a keyboard input and validates it

test(interaction): add interaction spec for mousedown event + formats 4 spaces to 2

style(interaction): fix styling of the previous commits

test(sidenav): add sidenav focus restore test triggered by keyboard

test(sidenav): add focus restore test for mouse and patch other one

fix(sidenav): test

fix(sidenav): replace deprecated methods, and remove aria label

test(sidenav): revert deprecated methods.
@ThomasBurleson ThomasBurleson added needs: review This PR is waiting on review from the team and removed needs: review This PR is waiting on review from the team labels Nov 24, 2015
@ThomasBurleson ThomasBurleson modified the milestone: 1.0-rc7 Nov 24, 2015
@@ -344,9 +346,9 @@ function SidenavDirective($mdMedia, $mdUtil, $mdConstant, $mdTheming, $animate,
// When the current `updateIsOpen()` animation finishes
promise.then(function(result) {

if ( !scope.isOpen ) {
if ( !scope.isOpen && triggeringElement && triggeringInteractionType && triggeringInteractionType === 'keyboard') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triggeringInteractionType && triggeringInteractionType === 'keyboard'

can be simplified to

triggeringInteractionType === 'keyboard'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good idea.

@devversion devversion force-pushed the fix/sidenav-focus-restore-service branch from d741eab to 620fa6f Compare March 3, 2016 16:28
@devversion devversion added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Mar 3, 2016
@devversion devversion assigned devversion and unassigned EladBezalel Mar 21, 2016
@ThomasBurleson ThomasBurleson modified the milestones: 1.0.6, 1.0.8 Apr 4, 2016
@devversion devversion force-pushed the fix/sidenav-focus-restore-service branch from 620fa6f to 28aa59a Compare April 7, 2016 16:52
@devversion devversion removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Apr 7, 2016
@devversion
Copy link
Member Author

#7965

@devversion devversion closed this Apr 9, 2016
@devversion devversion deleted the fix/sidenav-focus-restore-service branch April 21, 2016 11:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: review This PR is waiting on review from the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

md-toolbar and sidenav focus problems Ripple 'sticks' if using UI Router in the app under certain conditions
5 participants