-
Notifications
You must be signed in to change notification settings - Fork 698
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(network): string type in node/linkComponent #1078
Conversation
I can't tell why Happo failed, because I can't follow the Detail link. (It says, "Sign in with your airbnb.com account to view this page.") But, it seems likely it's detecting the changes I made (intentionally) to the |
Hey @jraymakers sorry for the delay, thanks for the fix! I agree this shouldn't be considered breaking since it was broken :) We recently added happo and there are a couple of demos that are flaky, apologies for making you think you did something wrong and that you can't access the report. This is what it's reporting for your change: It's a pretty big change aesthetically, I'm wondering if we could update the logic such that a single link is changed and a single node is changed. This would demonstrate the customizability and keep the aesthetics similar to before. It'd also be great to shift the bottom rectangle up to be visible in the gallery tile since it's clipped with the new y-offset. |
Sure, that makes sense. I'll make the customizations more subtle. |
@williaster I changed the example to just have one custom node and one custom link, and used styling that should fit in better with the defaults and the other gallery tiles. I also fixed the clipping issue. As an added bonus, the example now shows how to use custom node and link types with this generic component. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jraymakers amazing, it looks great! thanks for being open to iterating on styles 😄
Heads up I'm working on automating releases, I think this could be a good candidate for testing but it may take me a couple days to get it right.
🎉 This PR is included in version v1.6.1-alpha.5 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.6 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.7 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.8 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.9 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.10 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.11 of the packages modified 🎉 |
🎉 This PR is included in version v1.6.1-alpha.12 of the packages modified 🎉 |
🎉 This PR is included in version |
Fixes #939
💥 Breaking Changes
📝 Documentation
network
example to show use ofnodeComponent
andlinkComponent
, as well astop
andleft
props. Adapted from demo in comment of bug(network): string type is invalid for node/linkComponent #939🐛 Bug Fix
nodeComponent
andlinkComponent
props innetwork
package. This type was invalid in spite of being in the type signature. See bug(network): string type is invalid for node/linkComponent #939