Skip to content
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.

Sign up for GitHub

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

[BREAKING] Remove deprecations in anticipation of 3.0.0 #246

Merged
merged 2 commits into from
Mar 3, 2018
Merged

[BREAKING] Remove deprecations in anticipation of 3.0.0 #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d78c1e
[BREAKING] Remove deprecations in anticipation of 3.0.0
lukemelia Mar 3, 2018
984a155
Safer click target check
lukemelia Mar 3, 2018
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
6 changes: 2 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Property | Purpose
`overlayClassNames` | CSS class names to append to overlay divs. This is a concatenated property, so it does **not** replace the default overlay class (default: `'ember-modal-overlay'`. If you subclass this component, you may define this in your subclass.)
`wrapperClass` | CSS class name(s) to append to wrapper divs. Set this from template.
`wrapperClassNames` | CSS class names to append to wrapper divs. This is a concatenated property, so it does **not** replace the default container class (default: `'ember-modal-wrapper'`. If you subclass this component, you may define this in your subclass.)
`animatable` | A boolean, when `true` makes modal animatable using `liquid-fire` (requires `liquid-wormhole` to be installed, and for tethering situations `liquid-tether`. Having these optional dependencies installed and NOT explicitly specifying `animatable` is deprecated in 2.x and is equivalent to `animatable=false` for backwards compatibility. As of 3.x, the implicit default will be `animatable=true` when the optional `liquid-wormhole`/`liquid-tether` dependency is present.
`animatable` | A boolean, when `true` makes modal animatable using `liquid-fire` (requires `liquid-wormhole` to be installed, and for tethering situations `liquid-tether`. Having these optional dependencies installed and not specifying `animatable` will make `animatable=true` be the default.

#### Tethering

Expand Down Expand Up @@ -121,9 +121,7 @@ Property | Purpose

This component supports animation when certain addons are present (liquid-wormhole, liquid-tether).

_Current 2.x behavior:_ Having these optional dependencies installed and NOT explicitly specifying `animatable` is deprecated in 2.x and is equivalent to `animatable=false` for backwards compatibility. To get animation, pass `animatable=true` and install `liquid-wormhole` for non-tethering usage and `liquid-tether` for tethering usage.

_Upcoming 3.x behavior:_ Detection will be automatic. To opt out of using animatable features when you have these `liquid-*` addons installed, pass `animatable=false`.
Detection is be automatic. To opt out of using animatable features when you have these `liquid-*` addons installed, pass `animatable=false`.

When in an animatable scenario, you may also pass the following properties, which are passed through to liquid-wormhole or liquid-tether:

Expand Down
3 changes: 2 additions & 1 deletion addon/components/basic-dialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ export default Component.extend({
}

// if the click is within the dialog, do nothing
if (document.querySelector(modalSelector).contains(target)) {
let modalEl = document.querySelector(modalSelector);
if (modalEl && modalEl.contains(target)) {
Copy link

Choose a reason for hiding this comment

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

This is safer, but I'm not sure it's quite correct. If there's no modalEl, it still runs onClose. My impression is that it should be

let modalEl = document.querySelector(modalSelector);
if (!modelEl || modelEl.contains(target)) {
  return;
}

this.sendAction('onClose');

return;
}

Expand Down
136 changes: 0 additions & 136 deletions addon/components/deprecated-tether-dialog.js
View file
Open in desktop

This file was deleted.

22 changes: 0 additions & 22 deletions addon/components/modal-dialog-overlay.js
View file
Open in desktop

This file was deleted.

92 changes: 2 additions & 90 deletions addon/components/modal-dialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,14 @@
import Component from '@ember/component';
import { dasherize } from '@ember/string';
import { computed } from '@ember/object';
import { isNone, isEmpty } from '@ember/utils';
import { isEmpty } from '@ember/utils';
import { inject as service } from '@ember/service';
import layout from '../templates/components/modal-dialog';
import { deprecate, warn } from '@ember/debug';
import { warn } from '@ember/debug';
import { DEBUG } from '@glimmer/env';

const VALID_OVERLAY_POSITIONS = ['parent', 'sibling'];

function deprecateImplicitAnimatableWithLiquidTetherPresent() {
deprecate(
'Rendering modal-dialog with a tetherTarget and liquid-tether installed, and NOT explicitly specifying `animatable` will change behavior in 3.0.0 to use liquid-tether. Pass `animatable=false` to maintain current behavior and remove this message.',
false,
{ id: 'ember-modal-dialog.implicit-animatable', until: '3.0.0' }
);
}

function deprecateImplicitAnimatableWithLiquidWormholePresent() {
deprecate(
'Rendering modal-dialog with liquid-wormhole installed, and NOT explicitly specifying `animatable` will change behavior in 3.0.0 to use liquid-wormhole. Pass `animatable=false` to maintain current behavior and remove this message.',
false,
{ id: 'ember-modal-dialog.implicit-animatable', until: '3.0.0' }
);
}

export default Component.extend({
tagName: '',
layout,
Expand All @@ -38,18 +22,11 @@ export default Component.extend({

if (this.get('renderInPlace')) {
return 'ember-modal-dialog/-in-place-dialog';
} else if (tetherTarget && hasLiquidTether && hasLiquidWormhole && isNone(animatable)) {
deprecateImplicitAnimatableWithLiquidTetherPresent();
this.ensureEmberTetherPresent();
return 'ember-modal-dialog/-tether-dialog';
} else if (tetherTarget && hasLiquidTether && hasLiquidWormhole && animatable === true) {
return 'ember-modal-dialog/-liquid-tether-dialog';
} else if (tetherTarget) {
this.ensureEmberTetherPresent();
return 'ember-modal-dialog/-tether-dialog';
} else if (hasLiquidWormhole && isNone(animatable)) {
deprecateImplicitAnimatableWithLiquidWormholePresent();
return 'ember-modal-dialog/-basic-dialog';
} else if (hasLiquidWormhole && animatable === true) {
return 'ember-modal-dialog/-liquid-dialog';
}
Expand All @@ -76,66 +53,14 @@ export default Component.extend({
}
},
// onClose - set this from templates
close: computed('onClose', {
get() {
return this.get('onClose');
},
set(key, value) {
deprecate(
'Specifying the `close` action for a modal-dialog/tether-dialog is deprecated in favor of `onClose`. Will be removed in 3.0.0.',
false,
{ id: 'ember-modal-dialog.close-action', until: '3.0.0' }
);
this.set('onClose', value);
},
}),

// containerClass - set this from templates
"container-class": computed('containerClass', {
get() {
return this.get('containerClass');
},
set(key, value) {
deprecate(
'Passing `container-class` (kebab-case) is deprecated in favor of `containerClass` (camelCase). Will be removed in 3.0.0.',
false,
{ id: 'ember-modal-dialog.kebab-props', until: '3.0.0' }
);
this.set('containerClass', value);
},
}),
containerClassNames: ['ember-modal-dialog'], // set this in a subclass definition

// overlayClass - set this from templates
"overlay-class": computed('overlayClass', {
get() {
return this.get('overlayClass');
},
set(key, value) {
deprecate(
'Passing `overlay-class` (kebab-case) is deprecated in favor of `overlayClass` (camelCase). Will be removed in 3.0.0.',
false,
{ id: 'ember-modal-dialog.kebab-props', until: '3.0.0' }
);
this.set('overlayClass', value);
},
}),
overlayClassNames: ['ember-modal-overlay'], // set this in a subclass definition

// wrapperClass - set this from templates
"wrapper-class": computed('wrapperClass', {
get() {
return this.get('wrapperClass');
},
set(key, value) {
deprecate(
'Passing `wrapper-class` (kebab-case) is deprecated in favor of `wrapperClass` (camelCase). Will be removed in 3.0.0.',
false,
{ id: 'ember-modal-dialog.kebab-props', until: '3.0.0' }
);
this.set('wrapperClass', value);
},
}),
wrapperClassNames: ['ember-modal-wrapper'], // set this in a subclass definition

concatenatedProperties: ['containerClassNames', 'overlayClassNames', 'wrapperClassNames'],
Expand All @@ -148,19 +73,6 @@ export default Component.extend({
tetherTarget: null,
stack: computed.oneWay('elementId'), // pass a `stack` string to set a "stack" to be passed to liquid-wormhole / liquid-tether
value: 0, // pass a `value` to set a "value" to be passed to liquid-wormhole / liquid-tether
target: computed({ // element, css selector, or view instance
get() {
return 'body';
},
set(key, value) {
deprecate(
'Specifying a `target` on `modal-dialog` is deprecated in favor of padding `tetherTarget`, which will trigger ember-tether usage. Support for `target` will be removed in 3.0.0.',
false,
{ id: 'ember-modal-dialog.modal-dialog-target', until: '3.0.0' }
);
return value;
},
}),

targetAttachment: 'middle center',
tetherClassPrefix: null,
Expand Down
29 changes: 0 additions & 29 deletions addon/templates/components/deprecated-tether-dialog.hbs
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion app/components/modal-dialog-overlay.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion app/components/tether-dialog.js
View file
Open in desktop

This file was deleted.

Loading