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

fix: prevent overwriting htmlAttributes.ref #496

Merged
merged 4 commits into from
Nov 15, 2019

Conversation

sherwinski
Copy link
Contributor

@sherwinski sherwinski commented Nov 13, 2019

Fixes #494
Refactoring work completed in #475 inadvertently introduced a regression where we are overwriting a ref passed via htmlAttributes. This PR better handles this situation while accounting for refs that are either passed as a callback or an object.

@sherwinski
Copy link
Contributor Author

@jaredloson Feel free to take a look and leave feedback as well.

src/react-imgix.jsx Outdated Show resolved Hide resolved
Copy link
Contributor

@frederickfogerty frederickfogerty left a comment

Choose a reason for hiding this comment

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

This is a good start, but I think it's best to add another test to ensure htmlAttributes and onMounted play nicely together.

test/unit/react-imgix.test.jsx Outdated Show resolved Hide resolved
test/unit/react-imgix.test.jsx Show resolved Hide resolved
src/react-imgix.jsx Outdated Show resolved Hide resolved
src/react-imgix.jsx Outdated Show resolved Hide resolved
@commit-lint
Copy link

commit-lint bot commented Nov 14, 2019

Bug Fixes

  • callback refs overwritting htmlAttributes.ref (9d7abbf)

Styles

Contributors

@sherwinski

Copy link
Contributor

@frederickfogerty frederickfogerty 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 making those changes @sherwinski! I'd be happy to merge it in this state, but I think there's just a few little things we could improve if we want.

src/react-imgix.jsx Outdated Show resolved Hide resolved
test/unit/react-imgix.test.jsx Outdated Show resolved Hide resolved
test/unit/react-imgix.test.jsx Outdated Show resolved Hide resolved
@sherwinski sherwinski merged commit e15e1b2 into master Nov 15, 2019
@sherwinski sherwinski deleted the fix-htmlAttributes-refs branch November 15, 2019 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

htmlAttributes ref function not called
2 participants