Help me submit a PR to fix #targetAttachmentClass deprecation in Ember Modal Dialog

Tweeted @lukemelia a month ago asking if he planned to update ember-modal-dialog to remove this deprecation warning.

#targetAttachmentClass computed property was just overriden. This removes the computed property and replaces it with a plain value, and has been deprecated.

Was looking into it last night and saw someone submitted an issue about the deprecation around the same time. Told Luke if someone could guide me through it, I’d be happy to submit the PR. If you follow the link, you will see his reply:

Hi @jameshahn2, the reason for this error is that targetAttachmentClass is a computed property. If a value is passed in for the property, Ember’s historical behavior is that the passed value would replace the computed function. This is now deprecated, hence the warning. So we need a new way to provide a default value for targetAttachmentClass. I think that a way to do it would be to make the init method of the component(s) set targetAttachmentClass if and only if this.targetAttachmentClass is undefined.

After searching the repo for “target”, I narrowed the problematic files down to what you see below.

This is not necessarily every file that needs to be fixed and there could be far fewer problematic files than I linked to here. However, I’m hoping they will help y’all see what needs to happen. If a couple of you could take a look and tell me how to fix the issue, I will git clone the directory, make the changes, and submit the PR.

Let’s make ember-modal-dialog great again!

1 Like

Thanks for doing the legwork here @jameshahn2! Here is an example refactor of one of the targetAttachmentClass computed properties: https://github.com/yapplabs/ember-modal-dialog/compare/master...efx:refactor-computed?expand=1. I annotated the source with comments explaining each step. Let me know if that is clear and/or other tips to explain what is going on. This implements Lukes suggestion on the comment. It would need testing and applied to the other sections.

2 Likes

Makes sense to me. I’ll get on it this afternoon.

Thank you, sir!

1 Like

@jameshahn2 Any update on this?

1 Like

Goodness! Fell headlong through the Python, data science wormhole and forgot to make this happen. Working through a text summarization project I’ve been working on.

Will get this done today!

1 Like