feat: added blocklyHighlighted CSS class to highlighted block's root…
#8407
Conversation
Shreshthaaa
changed the title
blocklyHighlighted CSS class to highlighted block's root…
BeksOmega
left a comment
BeksOmega
left a comment
There was a problem hiding this comment.
Thanks for your work! Just one fix.
core/renderers/common/path_object.ts
Outdated
| addClass(element: SVGElement, className:string) { | ||
| element.classList.add(className); | ||
| } | ||
|
|
||
| removeClass(element: SVGElement, className: string) { | ||
| element.classList.remove(className); | ||
| } |
There was a problem hiding this comment.
Could you remove these and call the existing setClass method instead?
|
Heya @Shreshthaaa Are you still interested in working on this? (and your other open PRs?) |
Hi @BeksOmega , thank you for following up. I apologize for the delay. I've been quite busy lately but I am still interested in working on this and my other open PRs. I will address the requested changes in a day or two. |
|
@BeksOmega Hey, I have done the said changes. Kindly review. |
BeksOmega
left a comment
BeksOmega
left a comment
There was a problem hiding this comment.
Just pulled this down locally to test and I don't think this is working for the renders/geras/path_object. Can you refactor that to properly call super?
Hey, I made changes in |
BeksOmega
left a comment
BeksOmega
left a comment
There was a problem hiding this comment.
Thanks for the changes but that doesn't quite do what I asked! In the geras path object it should call super to add the new CSS class. The only new code in the gears path object should be setting the svgPathLight.style.display values.
Hey, I have called |
|
|
||
| override updateHighlighted(highlighted: boolean) { | ||
| super.updateHighlighted(highlighted); | ||
| if (highlighted) { |
There was a problem hiding this comment.
Still needs a change! Like I said the only lines should be the svgPathLight.style.display (and the associated if-else).
You can remove the lines in the geras path object that set the filter, since that already happens in the super function.
|
Heya @Shreshthaaa are you still interested in working on this? |
Hey @BeksOmega, please review the update. Hopefully, everything is in line now. Thank you! |
BeksOmega
left a comment
BeksOmega
left a comment
There was a problem hiding this comment.
Perfect! Thank you for the changes =) Once this passes CI I'll get it merged.
BeksOmega
changed the title
blocklyHighlighted CSS class to highlighted block's root…blocklyHighlighted CSS class to highlighted block's root…
github-actions
bot
added
PR: feature
3 participants
Details :
Added
blocklyHighlightedCSS class to highlighted block's root svg.Fixes:
#8263