Skip to content
This repository has been archived by the owner on May 7, 2019. It is now read-only.

Limits react-portal-tooltip to v2.3.0 #183

Closed
wants to merge 2 commits into from

Conversation

valadas
Copy link
Contributor

@valadas valadas commented Dec 15, 2018

The latest react-portal-tooltip version has an issue with the latest React version. This PR limits it to the latest compatible verion. Fixes errors with tooltips and allows the Security module (and probably other modules) to not fail because of this bug.

closes #182

@valadas valadas changed the title Limites react-portal-tooltip to v2.3.0 Limits react-portal-tooltip to v2.3.0 Dec 15, 2018
@valadas
Copy link
Contributor Author

valadas commented Dec 15, 2018

@mtrutledge Any oppinion on this workaround? Have a better idea on this or my solution makes sense for now?

@valadas
Copy link
Contributor Author

valadas commented Dec 15, 2018

Also if we can merge those 2 PRs on common, it would help with this module since I think it is affected by those two issues:

#183

#180

@ohine
Copy link
Contributor

ohine commented Dec 15, 2018

@valadas can you take a look at the conflict?

@valadas
Copy link
Contributor Author

valadas commented Dec 15, 2018

Oh, it is the minified result of the build, that will be re-generated by CI, should not even be tracked in my opinion... I think I can just remove it, hold on, I will try

@valadas
Copy link
Contributor Author

valadas commented Dec 15, 2018

Hmm, I deleted the file and pushed but it still sees that as a difference, not sure how to correct that. @ohine any advice? or you just cherry-pick it?

@ohine
Copy link
Contributor

ohine commented Dec 15, 2018

That's weird your delete didn't fix it. Sure thing, I nuke it and merge shortly. Thanks for giving it a try!

@valadas
Copy link
Contributor Author

valadas commented Dec 15, 2018

Yeah, if it existed before the PR and I delete it, it is still seen as a change, Ideally I should have just not included in the the commit, but not sure how to undo that.

@valadas
Copy link
Contributor Author

valadas commented Dec 17, 2018

I think we should delete the bundles on all those modules and add those filenames in the gitignore file. It will prevent those merge conflicts and the CI builds them anyway. Do you agree @mtrutledge ?

@valadas
Copy link
Contributor Author

valadas commented Dec 18, 2018

Please do not merge yet. Looks like that bug got fixed in their latest release. I will give it a try and submit another PR if it works...

@daguiler
Copy link
Contributor

Instead of deleting the file, try reverting all changes to that file in your PR @valadas, that way it will no longer cause any conflicts. And then create a new PR to remove the file.

@valadas
Copy link
Contributor Author

valadas commented Dec 19, 2018

Hmm, strange I commented this yesterday but it did not save. Please do not merge this, the issue might have been fixed in their latest release and this workaround not needed. Testing now, will closes if tests are conclusive.

@valadas
Copy link
Contributor Author

valadas commented Dec 19, 2018

Replaced by #184 I tested their new release and it works fine now, so the new solution is to upgrade instead of limit to an older version

@valadas valadas closed this Dec 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tooltip with info throws an error
4 participants