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.

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

Replace ember-ref-modifier to ember-ref-bucket #1182

Merged

Conversation

lifeart
Copy link
Contributor

@lifeart lifeart commented Aug 12, 2020

Since ember-ref-modifier has list of downsides (runloop scheduling), api design, I will suggest to replace it to new implementation.

https://github.com/lifeart/ember-ref-bucket

refs:
lifeart/ember-ref-modifier#345
lifeart/ember-ref-modifier#203
lifeart/ember-ref-modifier#232

@simonihmig
Copy link
Contributor

@lifeart thanks for working on this! I haven't looked deeply into ref-bucket yet, but wonder if the error could be caused by having both ember-ref-bucket and ember-ref-modifier as dependencies, as that would cause a name clash, right? This addon also uses ember-popper, which still uses ref-modifier (refactored away in kybishop/ember-popper#127, but this has not landed here yet).

Shouldn't both addons be able to co-exist? This would mean to rename the {{ref}} modifier in the new one I guess!?

@lifeart
Copy link
Contributor Author

lifeart commented Aug 13, 2020

@simonihmig

Shouldn't both addons be able to co-exist? This would mean to rename the {{ref}} modifier in the new one I guess!?

I whould like to have only one ref-* addon, because ref-modifier is kinda legacy :)
I'm agree on different naming for ref-bucket implementation, do you have any naming suggestions?

ref-as -> may confuse developers fabiliar with blocks {{#let foo as ...}};

ref-to - already used as helper, and I think we could use it as modifier name

ref-name - may be an alternative name for modifier.

create-ref - may be an case (to follow react naming https://reactjs.org/docs/refs-and-the-dom.html#creating-refs)

@basz
Copy link
Contributor

basz commented Aug 14, 2020

{{ref this "_element"}} is used in 6 places and is currently problematic...

@lifeart
Copy link
Contributor Author

lifeart commented Aug 14, 2020

ember-ref-bucket now released under v1.0.0, modifier name replaced from ref to create-ref, to keep backward compatibility with ember-ref-modifier.
CI looks green

@basz
Copy link
Contributor

basz commented Aug 14, 2020

Cool! Nice work!

@basz
Copy link
Contributor

basz commented Aug 19, 2020

May I kindly ping for a release?

Copy link
Contributor

@simonihmig simonihmig left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! 👍

Looks good, just two little things to fix...

addon/templates/components/bs-form.hbs Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@lifeart lifeart force-pushed the improve-replace-ember-ref-modifier branch from c4017ab to 7338d32 Compare August 19, 2020 15:11
Copy link
Contributor

@simonihmig simonihmig left a comment

Choose a reason for hiding this comment

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

Thank you!

@simonihmig simonihmig merged commit e47e4ca into ember-bootstrap:master Aug 19, 2020
@simonihmig simonihmig changed the title improve: replace ember-ref-modifier to ember-ref-bucket Replace ember-ref-modifier to ember-ref-bucket Aug 19, 2020
@lifeart lifeart deleted the improve-replace-ember-ref-modifier branch August 20, 2020 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants