Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix:- Fixed dev tools inspect mode on Shadow dom #26888

Merged
merged 4 commits into from
Jun 7, 2023
Merged

Fix:- Fixed dev tools inspect mode on Shadow dom #26888

merged 4 commits into from
Jun 7, 2023

Conversation

Biki-das
Copy link
Contributor

@Biki-das Biki-das commented Jun 1, 2023 •
edited
Loading

Fixes #26200

PR explanation

I tried to induce the change by the event.composed to check whether the event was created in a ShadowRoot, And replaced pointerOver with pointerMove, pointerOver event did not fired correctly

Before PR:-

Screen.Recording.2023-06-01.at.11.42.34.PM.mov

After PR:-

Screen.Recording.2023-06-01.at.11.45.37.PM.mov

@Biki-das
Copy link
Contributor Author

Biki-das commented Jun 1, 2023 •
edited
Loading

cc @mondaychen @hoxyq

hoxyq
hoxyq requested changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@hoxyq hoxyq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Biki-das, thanks for your contribution!

Overall, I am okay with this solution, but some small changes/discussions required to proceed

packages/react-devtools-shared/src/backend/views/Highlighter/index.js Show resolved Hide resolved
packages/react-devtools-shared/src/backend/views/Highlighter/index.js Show resolved Hide resolved
packages/react-devtools-shared/src/backend/views/Highlighter/index.js Outdated Show resolved Hide resolved
@Biki-das Biki-das requested a review from hoxyq June 5, 2023 11:18
@Biki-das
Copy link
Contributor Author

Biki-das commented Jun 6, 2023

@hoxyq ping!

@Biki-das Biki-das requested a review from hoxyq June 7, 2023 15:33
@hoxyq hoxyq merged commit 9100456 into facebook:main Jun 7, 2023
@Biki-das Biki-das deleted the devtools-patch branch June 7, 2023 19:28
@Jack-Works
Copy link
Contributor

I made the same bug fix last year. See #25518

Why my PR is totally ignored by the React team but this PR gets merged in one week? I feel very uncomfortable about this. 🙁

@Biki-das
Copy link
Contributor Author

I made the same bug fix last year. See #25518

Why my PR is totally ignored by the React team but this PR gets merged in one week? I feel very uncomfortable about this. 🙁

😅 whoa! We had the same approach to this problem! @Jack-Works , really feeling bad for this, the react team actually is very busy to respond, i just happened to tag a few people around the team who were actively making changes!

@Jack-Works
Copy link
Contributor

I made the same bug fix last year. See #25518
Why my PR is totally ignored by the React team but this PR gets merged in one week? I feel very uncomfortable about this. 🙁

😅 whoa! We had the same approach to this problem! @Jack-Works , really feeling bad for this, the react team actually is very busy to respond, i just happened to tag a few people around the team who were actively making changes!

Yes, we also use ShadowDOM a lot. Currently, we're patching the react-devtools in our node_modules for months.

I often go to all PRs I made in the big project to see if there is a merge conflict so I can resolve it, then I found my PR conflicts today.

I'm happy to see this issue get fixed so I can remove the patch, but I feel hurt about the team's community governance.

@Jack-Works Jack-Works mentioned this pull request Jun 10, 2023
Closed
@hoxyq
Copy link
Contributor

hoxyq commented Jun 10, 2023

I made the same bug fix last year. See #25518

Why my PR is totally ignored by the React team but this PR gets merged in one week? I feel very uncomfortable about this. 🙁

Hey @Jack-Works, I am sorry about that. The issue #25519 was overlooked for a long time, I just started to maintain DevTools around 2 months ago. It is just a coincidence that @Biki-das tagged me to review his PR.

We really value any contributions, feel free to tag me in any issues / PRs related to DevTools. It might take some time for me to respond, but this will be in my queue.

This fix should be included in next React DevTools release, which I am planning to ship in the next 1-2 weeks.

@hoxyq hoxyq mentioned this pull request Jun 10, 2023
Closed
@styfle styfle mentioned this pull request Jun 30, 2023
Merged
@hoxyq hoxyq mentioned this pull request Jul 4, 2023
Merged
hoxyq added a commit that referenced this pull request Jul 4, 2023
@hoxyq
React DevTools 4.27.8 -> 4.28.0 (#27051)
035a41c 
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [#27032](#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[#27008](#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[#26997](#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[#26955](#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[#26963](#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[#26983](#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[#26951](#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[#26887](#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[#26947](#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[#25510](#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[#26888](#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[#26830](#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [#26825](#26825))
timneutkens pushed a commit to vercel/next.js that referenced this pull request Jul 4, 2023
@styfle @shuding
chore(deps): bump react 18.3.0-canary-1fdacbefd-20230630 (#52005)
9e4b87d 
This PR updates the vendored react dependencies using `pnpm sync-react`

### React upstream changes

- facebook/react#27028
- facebook/react#27027
- facebook/react#27019
- facebook/react#26954
- facebook/react#26987
- facebook/react#26985
- facebook/react#26933
- facebook/react#26625
- facebook/react#27011
- facebook/react#27008
- facebook/react#26997
- facebook/react#26989
- facebook/react#26955
- facebook/react#26963
- facebook/react#26983
- facebook/react#26914
- facebook/react#26951
- facebook/react#26977
- facebook/react#26958
- facebook/react#26940
- facebook/react#26939
- facebook/react#26887
- facebook/react#26947
- facebook/react#26945
- facebook/react#26942
- facebook/react#26938
- facebook/react#26844
- facebook/react#25510
- facebook/react#26932
- facebook/react#26896
- facebook/react#26913
- facebook/react#26888
- facebook/react#26827
- facebook/react#26889
- facebook/react#26877
- facebook/react#26873
- facebook/react#26880
- facebook/react#26842
- facebook/react#26858
- facebook/react#26754
- facebook/react#26753
- facebook/react#26881

### Related

Closes #49409 (by facebook/react#26977)
fix NEXT-1189

Co-authored-by: Shu Ding <g@shud.in>
@styfle styfle mentioned this pull request Jul 5, 2023
Merged
timneutkens pushed a commit to vercel/next.js that referenced this pull request Jul 6, 2023
@styfle @huozhi
chore(deps): bump react to 7118f5dd7 (#52282)
6dafc42 
Fixes #49409

### React upstream changes

- facebook/react#27045
- facebook/react#27051
- facebook/react#27032
- facebook/react#27031
- facebook/react#27029
- facebook/react#27028
- facebook/react#27027
- facebook/react#27019
- facebook/react#26954
- facebook/react#26987
- facebook/react#26985
- facebook/react#26933
- facebook/react#26625
- facebook/react#27011
- facebook/react#27008
- facebook/react#26997
- facebook/react#26989
- facebook/react#26955
- facebook/react#26963
- facebook/react#26983
- facebook/react#26914
- facebook/react#26951
- facebook/react#26977
- facebook/react#26958
- facebook/react#26940
- facebook/react#26939
- facebook/react#26887
- facebook/react#26947
- facebook/react#26945
- facebook/react#26942
- facebook/react#26938
- facebook/react#26844
- facebook/react#25510
- facebook/react#26932
- facebook/react#26896
- facebook/react#26913
- facebook/react#26888
- facebook/react#26827
- facebook/react#26889
- facebook/react#26877
- facebook/react#26873
- facebook/react#26880
- facebook/react#26842
- facebook/react#26858
- facebook/react#26754
- facebook/react#26753
- facebook/react#26881

---------

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
shuding added a commit to vercel/next.js that referenced this pull request Jul 8, 2023
@styfle @shuding
chore(deps): bump react 18.3.0-canary-1fdacbefd-20230630 (#52005)
1ae3f42 
This PR updates the vendored react dependencies using `pnpm sync-react`

### React upstream changes

- facebook/react#27028
- facebook/react#27027
- facebook/react#27019
- facebook/react#26954
- facebook/react#26987
- facebook/react#26985
- facebook/react#26933
- facebook/react#26625
- facebook/react#27011
- facebook/react#27008
- facebook/react#26997
- facebook/react#26989
- facebook/react#26955
- facebook/react#26963
- facebook/react#26983
- facebook/react#26914
- facebook/react#26951
- facebook/react#26977
- facebook/react#26958
- facebook/react#26940
- facebook/react#26939
- facebook/react#26887
- facebook/react#26947
- facebook/react#26945
- facebook/react#26942
- facebook/react#26938
- facebook/react#26844
- facebook/react#25510
- facebook/react#26932
- facebook/react#26896
- facebook/react#26913
- facebook/react#26888
- facebook/react#26827
- facebook/react#26889
- facebook/react#26877
- facebook/react#26873
- facebook/react#26880
- facebook/react#26842
- facebook/react#26858
- facebook/react#26754
- facebook/react#26753
- facebook/react#26881

### Related

Closes #49409 (by facebook/react#26977)
fix NEXT-1189

Co-authored-by: Shu Ding <g@shud.in>
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@Biki-das @AndyPengc12
Fix:- Fixed dev tools inspect mode on Shadow dom (facebook#26888)
3801fc3 
Fixes facebook#26200 

### PR explanation

I tried to induce the change by the `event.composed` to check whether
the event was created in a ShadowRoot, And replaced `pointerOver` with
`pointerMove`, pointerOver event did not fired correctly

Before PR:-


https://github.com/facebook/react/assets/72331432/67a33dcd-447f-4c68-9c3c-ad954baddeb8

After PR:-


https://github.com/facebook/react/assets/72331432/9f986ff2-785f-4cba-a504-44f82ea9fc5a

---------

Co-authored-by: Biki das <bikidas@Bikis-MacBook-Pro.local>
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@hoxyq @AndyPengc12
React DevTools 4.27.8 -> 4.28.0 (facebook#27051)
ce7fcbb 
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [facebook#27032](facebook#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[facebook#27008](facebook#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[facebook#26997](facebook#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[facebook#26955](facebook#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[facebook#26963](facebook#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[facebook#26983](facebook#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[facebook#26951](facebook#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[facebook#26887](facebook#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[facebook#26947](facebook#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[facebook#25510](facebook#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[facebook#26888](facebook#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[facebook#26830](facebook#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [facebook#26825](facebook#26825))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
@hoxyq
Fix:- Fixed dev tools inspect mode on Shadow dom (#26888)
8ed315a 
Fixes #26200

### PR explanation

I tried to induce the change by the `event.composed` to check whether
the event was created in a ShadowRoot, And replaced `pointerOver` with
`pointerMove`, pointerOver event did not fired correctly

Before PR:-

https://github.com/facebook/react/assets/72331432/67a33dcd-447f-4c68-9c3c-ad954baddeb8

After PR:-

https://github.com/facebook/react/assets/72331432/9f986ff2-785f-4cba-a504-44f82ea9fc5a

---------

Co-authored-by: Biki das <bikidas@Bikis-MacBook-Pro.local>

DiffTrain build for commit 9100456.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoxyq hoxyq hoxyq approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: React dev tools inspect element of hover does not work on shadow elements
4 participants
@Biki-das @Jack-Works @hoxyq @facebook-github-bot