bvaughn / react-devtools-experimental

Experimental rewrite of the React DevTools extension
https://react-devtools-experimental.now.sh
MIT License
965 stars 55 forks source link

Inspect matching DOM element button should not open the Elements tab #304

Closed bvaughn closed 5 years ago

bvaughn commented 5 years ago

The inspect button (the 👁 button) currently does the following:

I've gotten feedback from multiple people that the latter action is a little jarring. I think we should just remove it.

fanny commented 5 years ago

Hi @bvaughn, i'm interested on this feature, but i can't reproduce this last step. Do you can explain me better? I think that a print would help me.

bvaughn commented 5 years ago

Check out the video below: inspect-Kapture 2019-07-17 at 13 54 16

This issue is saying that the Elements tab shouldn't be auto-opened.

fanny commented 5 years ago

Oh, now, i understand. Many thanks, i will check out the code, to understand how can i change this behaviour.

fanny commented 5 years ago

If i analyzed the correct files, the problem is here: https://github.com/bvaughn/react-devtools-experimental/blob/9fb753b5cd399ccc702bd3e53dd8d86a739e3504/src/devtools/views/Components/SelectedElement.js#L61-L68

In line 65, the openNativeElementsPanel is set as true, following the request, the correct is false. I did the build and just with this change, the behavior was changed, is that right? If yes, i can open pr fixing this.

bvaughn commented 5 years ago

Hmm... maybe, although I think that will also have the effect of not syncing the selection between the two panels (browser Elements and React Components). It would be nice if we were able to sync them- so if you switched to Elements intentionally, e.g. to edit a CSS prop, the right element would be selected for you. I just think the automatic tab change is not great.

Can we find a way to sync selection without the tab change? 😄

fanny commented 5 years ago

You're right. After this, we lost the ability to sync them. this because, the bridge event that sync both tabs, is activated by this prop.

https://github.com/bvaughn/react-devtools-experimental/blob/0f2fb5badf5908ee271e0b9e7ec7f3022e31a648/src/backend/views/Highlighter/index.js#L82-L85

So, the bridge invoke this method: https://github.com/bvaughn/react-devtools-experimental/blob/cb3fb4212972ad514ff53bff96530a0dffcaabcf/shells/browser/shared/src/main.js#L191-L205

But, as you see in the comment(and if you test in console of your browser the command inspect($0)), the inspect method helper doesn't have the hability of select a node whithout change the tab.

In this moment, i'm checking if exists a way for select the element without use inspect .

bvaughn commented 5 years ago

the inspect method helper doesn't have the hability of select a node whithout change the tab.

Yeah, I also don't know of a way around this limitation 😦 If you could find one, that would be sweet!

Maybe there would be a way for us to wait to sync the selection until the user opened the Elements tab? Not sure if this would be possible either.

fanny commented 5 years ago

The way that i think for make this, is detecting when the Elements panel is selected, the problem is that in elements panel docs the only methods that we can use are: create, that creates a panel and onSelectedChange that is fired if a user select a node. Differently of the user extension panel api, that have a onShow method that is fired when user switches to the panel.

The thing more similar to the we can do, is check if user open the devtool, i find this repo.

bvaughn commented 5 years ago

It might just be that this isn't possible to do then :smile: in which case, we could close this issue at least knowing that we tried.

fanny commented 5 years ago

yeah, sure! :laughing:

bvaughn commented 5 years ago

Okay I'm going to close this for now, because it seems like we don't really have the option of implementing this feature for the time being. Thanks for looking into it~