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

fix(speedDial): Don't intercept spaces and fix animations. #5396

Closed
wants to merge 1 commit into from
Closed

fix(speedDial): Don't intercept spaces and fix animations. #5396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 34 additions & 32 deletions src/components/fabSpeedDial/fabController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@

function setupListeners() {
var eventTypes = [
'$md.pressdown',

'click', // Fired via keyboard ENTER

'focusin', 'focusout'
'click', 'focusin', 'focusout'
];

// Add our listeners
Expand All @@ -68,34 +64,25 @@
});
}

var recentEvent;
var closeTimeout;
function parseEvents(event) {
// If we've had a recent press/click event, or material is sending us an additional event,
// ignore it
if (recentEvent && (isClick(recentEvent) || recentEvent.$material)) {
return;
}

// Otherwise, handle our events
if (isClick(event)) {
// If the event is a click, just handle it
if (event.type == 'click') {
handleItemClick(event);

// Store our recent click event
recentEvent = event;
} else if (event.type == 'focusin') {
vm.open();
} else if (event.type == 'focusout') {
vm.close();
}

// Clear the recent event after all others have fired so we stop ignoring
$timeout(function() {
recentEvent = null;
}, 100, false);
}
// If we focusout, set a timeout to close the element
if (event.type == 'focusout' && !closeTimeout) {
closeTimeout = $timeout(function() {
vm.close();
}, 100, false);
}

function isClick(event) {
return event.type == '$md.pressdown' || event.type == 'click';
// If we see a focusin and there is a timeout about to run, cancel it so we stay open
if (event.type == 'focusin' && closeTimeout) {
$timeout.cancel(closeTimeout);
closeTimeout = null;
}
}

function resetActionIndex() {
Expand Down Expand Up @@ -154,19 +141,34 @@
}

function enableKeyboard() {
angular.element(document).on('keydown', keyPressed);
$element.on('keydown', keyPressed);
angular.element(document).on('click', checkForOutsideClick);
angular.element(document).on('touchend', checkForOutsideClick);

// TODO: On desktop, we should be able to reset the indexes so you cannot tab through
// TODO: On desktop, we should be able to reset the indexes so you cannot tab through, but
// this breaks accessibility, especially on mobile, since you have no arrow keys to press
//resetActionTabIndexes();
}

function disableKeyboard() {
angular.element(document).off('keydown', keyPressed);
$element.off('keydown', keyPressed);
angular.element(document).off('click', checkForOutsideClick);
angular.element(document).off('touchend', checkForOutsideClick);
}

function checkForOutsideClick(event) {
if (event.target) {
var closestTrigger = $mdUtil.getClosest(event.target, 'md-fab-trigger');
var closestActions = $mdUtil.getClosest(event.target, 'md-fab-actions');

if (!closestTrigger && !closestActions) {
vm.close();
}
}
}

function keyPressed(event) {
switch (event.which) {
case $mdConstant.KEY_CODE.SPACE: event.preventDefault(); return false;
case $mdConstant.KEY_CODE.ESCAPE: vm.close(); event.preventDefault(); return false;
case $mdConstant.KEY_CODE.LEFT_ARROW: doKeyLeft(event); return false;
case $mdConstant.KEY_CODE.UP_ARROW: doKeyUp(event); return false;
Expand Down
23 changes: 17 additions & 6 deletions src/components/fabSpeedDial/fabSpeedDial.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
(function() {
'use strict';

/**
* The duration of the CSS animation in milliseconds.
*
* @type {number}
*/
var cssAnimationDuration = 300;

/**
* @ngdoc module
* @name material.components.fabSpeedDial
Expand Down Expand Up @@ -103,7 +110,9 @@
}
}

function MdFabSpeedDialFlingAnimation() {
function MdFabSpeedDialFlingAnimation($timeout) {
function delayDone(done) { $timeout(done, cssAnimationDuration, false); }

function runAnimation(element) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you manually setting the style animation properties?
Why not use $animateCSS( )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried switching to $animateCSS() during my last bout of fixing bugs, but I seem to remember running into some issues and after about an hour of playing with it I reverted back to the current method.

If you think it's worth the time, I am sure I can update it, but this PR was more concerned with getting it functional again after the gestures update :-) Perhaps we should create a separate issue to track updating this?

Copy link
Contributor

Choose a reason for hiding this comment

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

No need... I was simply curious.

It is my goal that the ngMaterial2 implementatioins should require consistent uses of gesture listeners and uses of $animateCss( ), or element.style({ ... });.

var el = element[0];
var ctrl = element.controller('mdFabSpeedDial');
Expand Down Expand Up @@ -169,17 +178,19 @@
addClass: function(element, className, done) {
if (element.hasClass('md-fling')) {
runAnimation(element);
done();
}
delayDone(done);
},
removeClass: function(element, className, done) {
runAnimation(element);
done();
delayDone(done);
}
}
}

function MdFabSpeedDialScaleAnimation() {
function MdFabSpeedDialScaleAnimation($timeout) {
function delayDone(done) { $timeout(done, cssAnimationDuration, false); }

var delay = 65;

function runAnimation(element) {
Expand Down Expand Up @@ -210,12 +221,12 @@
return {
addClass: function(element, className, done) {
runAnimation(element);
done();
delayDone(done);
},

removeClass: function(element, className, done) {
runAnimation(element);
done();
delayDone(done);
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/components/fabSpeedDial/fabSpeedDial.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ md-fab-speed-dial {

&.md-is-open {
.md-fab-action-item {
visibility: visible;
align-items: center;
}
}
Expand All @@ -43,7 +42,6 @@ md-fab-speed-dial {
height: auto;

.md-fab-action-item {
visibility: hidden;
transition: $swift-ease-in;
}
}
Expand Down Expand Up @@ -108,6 +106,15 @@ md-fab-speed-dial {
}
}

/*
* Hide some graphics glitches if switching animation types
*/
&.md-fling-remove, &.md-scale-remove {
.md-fab-action-item > * {
visibility: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use display: none; ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The speed dial currently uses flex to order/align both the trigger and the actions. When I used display: none with the speed dial in direction up, the actions height changed and the trigger jumped into the incorrect position :/

I think ideally we'll rewrite this in material2 to use absolute positioning that is calculated in the JS so we don't have to worry about issues like this.

}
}

/*
* Handle the animations
*/
Expand Down
56 changes: 6 additions & 50 deletions src/components/fabSpeedDial/fabSpeedDial.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('<md-fab-speed-dial> directive', function() {

it('toggles the menu when the trigger clicked', inject(function() {
build(
'<md-fab-speed-dial>' +
'<md-fab-speed-dial>' +
' <md-fab-trigger>' +
' <md-button></md-button>' +
' </md-fab-trigger>' +
Expand Down Expand Up @@ -107,55 +107,7 @@ describe('<md-fab-speed-dial> directive', function() {
expect(controller.isOpen).toBe(false);
}));

it('opens the menu when the trigger is focused', inject(function() {
build(
'<md-fab-speed-dial>' +
' <md-fab-trigger>' +
' <md-button></md-button>' +
' </md-fab-trigger>' +
'</md-fab-speed-dial>'
);

var focusEvent = {
type: 'focusin',
target: element.find('md-fab-trigger').find('md-button')
};

element.triggerHandler(focusEvent);
pageScope.$digest();
expect(controller.isOpen).toBe(true);
}));

it('closes the menu when the trigger is blurred', inject(function() {
build(
'<md-fab-speed-dial>' +
' <md-fab-trigger>' +
' <md-button></md-button>' +
' </md-fab-trigger>' +
'</md-fab-speed-dial>'
);

var focusInEvent = {
type: 'focusin',
target: element.find('md-fab-trigger').find('md-button')
};

var focusOutEvent = {
type: 'focusout',
target: element.find('md-fab-trigger').find('md-button')
};

element.triggerHandler(focusInEvent);
pageScope.$digest();
expect(controller.isOpen).toBe(true);

element.triggerHandler(focusOutEvent);
pageScope.$digest();
expect(controller.isOpen).toBe(false);
}));


it('properly finishes the fling animation', inject(function(mdFabSpeedDialFlingAnimation) {
it('properly finishes the fling animation', inject(function(mdFabSpeedDialFlingAnimation, $timeout) {
build(
'<md-fab-speed-dial md-open="isOpen" class="md-fling">' +
' <md-fab-trigger><button></button></md-fab-trigger>' +
Expand All @@ -167,9 +119,11 @@ describe('<md-fab-speed-dial> directive', function() {
var removeDone = jasmine.createSpy('removeDone');

mdFabSpeedDialFlingAnimation.addClass(element, 'md-is-open', addDone);
$timeout.flush();
expect(addDone).toHaveBeenCalled();

mdFabSpeedDialFlingAnimation.removeClass(element, 'md-is-open', removeDone);
$timeout.flush();
expect(removeDone).toHaveBeenCalled();
}));

Expand All @@ -185,9 +139,11 @@ describe('<md-fab-speed-dial> directive', function() {
var removeDone = jasmine.createSpy('removeDone');

mdFabSpeedDialScaleAnimation.addClass(element, 'md-is-open', addDone);
$timeout.flush();
expect(addDone).toHaveBeenCalled();

mdFabSpeedDialScaleAnimation.removeClass(element, 'md-is-open', removeDone);
$timeout.flush();
expect(removeDone).toHaveBeenCalled();
}));

Expand Down