Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Bug 1463780 - Set cursor style to element-node rep #6444

Merged
merged 2 commits into from May 30, 2018

Conversation

dadaa
Copy link
Contributor

@dadaa dadaa commented May 30, 2018

Summary of Changes

  • Set pointer cursor to .objectBox-node element in case of the element was clickable
  • Set pointer cursor to .open-inspector element

This changes are for https://bugzilla.mozilla.org/show_bug.cgi?id=1463780

Test Plan

Please run yarn test

@nchevobbe
Copy link
Member

Thanks @dadaa, this looks good.
Should we also add the clickable class to the text-node Rep too ?

@dadaa
Copy link
Contributor Author

dadaa commented May 30, 2018

Hi @nchevobbe !
In animation inspector, we are using only element-node.
(Also this change affects to FlexboxItem and GridItem[1])
However, if we need, I'll do that! (maybe another PR?). Please let me know if we need.

[1] https://searchfox.org/mozilla-central/search?q=defaultRep%3A+ElementNode%2C&case=false&regexp=false&path=

@nchevobbe
Copy link
Member

Yes, we should do it for TextNode as well, since they can be displayed in the console/debugger and we should be consistent about how we render similar things (e.g. text node can also be clicked to select them in the inspector).

@nchevobbe nchevobbe merged commit 63d2142 into firefox-devtools:master May 30, 2018
@dadaa
Copy link
Contributor Author

dadaa commented May 30, 2018

Ok! Will make the PR for text-node as well.
May I make also the cursor style of text-node to be same?

@nchevobbe
Copy link
Member

yes, it should be the same

@dadaa
Copy link
Contributor Author

dadaa commented May 30, 2018

Hi @nchevobbe !
I have looked the text-node. However I think that we could not append clickable, since the component does not have onDOMNodeClick which I had add last time.
Otherwise I think that we could not detect whether the element has click event listeners.

@nchevobbe
Copy link
Member

I have looked the text-node. However I think that we could not append clickable, since the component does not have onDOMNodeClick which I had add last time.

Oh okay then, I guess we only have the click on the icon.
Thanks for looking

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.

None yet

2 participants