-
Notifications
You must be signed in to change notification settings - Fork 12
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: reset code snippet when try to append a new code #352
Conversation
@@ -14,7 +14,7 @@ export const TimelineSourceIcon = styled.div<{ | |||
transition: background-color 150ms ease-out; | |||
|
|||
:before { | |||
content: "${({ kind }) => kind[0].toUpperCase()}"; | |||
content: "${({ kind }) => kind && kind[0].toUpperCase()}"; |
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.
Was this causing an issue?
Looking at the type definition this condition is always truthy
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.
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.
Hmmm - I don't think this is a bug here, it sounds like we're ending up with invalid data.
Are you able to try and catch the event that we're sourcing the kind
prop from. A console log somewhere around here should help us work out why we're missing the needed property
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.
If you want, lets revert the fix here for now so we can get this PR merged and then make a separate PR/issue for this particular problem!
It's a good find either way 🚀 we need to be able to trust our types so lets find the source 🕵️
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.
Reverted, I will investigate more. Thanks @andyrichardson
if (ref.hasChildNodes()) { | ||
ref.innerHTML = ""; | ||
} | ||
|
||
ref.appendChild(child); |
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.
So simple 😄 Love it!
I'll test it out in chrome now 👍
Edit: Works a charm 🚀
The solution: Clean the code section before appending new clicked query. We first check if there are some child nodes and if there are, we remove them and append the new code. It's strange that there are methods for getting firstChild, all children or parent element, but there is no method for cleaning html code inside the node.
Root cause: The main issue was that we are replace only the first child, which is one of many inside the node.
Issue: #351
Video after the fix:
Screen.Recording.2021-04-21.at.23.37.09.mov