Expanding objects makes them jump around
Categories
(DevTools :: Shared Components, defect, P3)
Tracking
(firefox88 fixed)
| Tracking | Status | |
|---|---|---|
| firefox88 | --- | fixed |
People
(Reporter: Harald, Assigned: karthiksundar30092002)
References
Details
(Keywords: good-first-bug)
Attachments
(8 files)
ScreenFlow.gif
Expanding objects and arrays changes their representation, often to something shorter. visual jitter.
Inspection should aim to keep objects in place.
|
Reporter | |
Comment 1•3 years ago
|
||
Nicolas, is there some prior arguments on why expanded objects change shape?
The argument was made in the very beginning of the ObjectInspector, and I can't find the discussion anymore .
I agree that it can be weird sometimes.
This should be a simple fix if we want to do it: remove this block devtools/client/debugger/packages/devtools-reps/src/object-inspector/components/ObjectInspectorItem.js#168-170 and fix the tests that would fail because of this change.
|
|
Reporter | |
Comment 3•3 years ago
|
||
This will probably need some hands-on experimentation to get right. Opening up for good-first-bug in case somebody who likes to tinker in UI wants to play around with this.
Different use cases should be considered (single entries, multiple entries in one log, narrow and wide viewport) and the amount of content jumping erratically reduced when expanding arrays/objects/etc.
|
|
Reporter | |
Comment 5•3 years ago
|
||
Thank you Samarjeet. Nicolas pointed to the right files that should get you started. As this might not be straightforward, feel free to share video (like Loom) recordings of any ideas you want to bounce off.
I can't find an object to reproduce it. Tried many levels of nesting. Could you explain how to reproduce this bug?
|
|
Reporter | |
Comment 7•3 years ago
|
||
In this example, clicking the Array pushes the Object around. Not shown, a smaller width expanding the Object pushes it down one line.
Other STR:
- Log
console.log(window, document) - Resize the console so both elements fit on the same line
- Expand one
AR: Clicked Objects jump around.
ER: Layout stays more stable
|
|
Reporter | |
Comment 10•3 years ago
|
||
I thought that it would be easiest if the Array(3) [ 1, 2, 3 ] would stay in place while additional content expands below.
The forced line break from comment 9 also makes sense, ensuring expanded objects get the space they need at all times.
Samarjeet, are you still interested in fixing this bug?
Honza
|
||
Comment 13•2 years ago
|
||
Hi, actually I am working on another bug at the moment. I don't think it's ethical for me to be working on two bugs at a time, it'll just delay it.
Let's open this up for contributors. I'll try getting back to this once the other bug is done.
Thanks for the heads-up Samarjeet, I unassigned the bug.
Hello Mary, the bug is now yours
If it's your first bug, you might want to read this https://firefox-source-docs.mozilla.org/devtools/getting-started/README.html to setup the work environment.
Make sure to select "Artifact builds" when asked as it speeds up the workflow quite a bit :)
Also, you can come and chat on our Slack https://devtools-html-slack.herokuapp.com/
When doing changes for that patch, you'll need to re-generate the bundle we have each time you're testing a change:
cd devtools/client/debugger
node bin/bundle
don't hesitate to ask any question here or on Slack
|
||
Comment 17•2 years ago
|
||
Thanks for the info Nick!
|
|
||
Comment 18•2 years ago
|
||
Hey Nick,
I ran into this error when I used the re-generate bundle code. I don't know where to find devtools-launchpad/index.
Hello Mary,
Could you try to do yarn install before? We need to pull those dependencies from npm :)
|
|
||
Comment 20•2 years ago
|
||
Hey Nick,
yarn install worked! I pushed a commit and added you as the reviewer. My only question now is, is there a revision ID for this bug?
Hello Mary,
I don't see the patch anywhere? What did you use to push?
Is the commit message like Bug 1608571 - Don't shorten object when expanding. r=nchevobbe. ?
|
|
||
Comment 22•2 years ago
|
||
I used hg commit and the message is Bug 1608571 - Fixes expanding Object issue. r=nchevobbe. I realized I forgot to use moz-phab submit, but now when I try to use it I get an API error.
can you try to run moz-phab install-certificate again?
|
||
Comment 24•2 years ago
|
||
This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.
|
Assignee | |
Comment 25•2 years ago
|
||
Can I take this bug??, If it is still open
|
|
Assignee | |
Comment 26•2 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
The argument was made in the very beginning of the ObjectInspector, and I can't find the discussion anymore .
I agree that it can be weird sometimes.This should be a simple fix if we want to do it: remove this block devtools/client/debugger/packages/devtools-reps/src/object-inspector/components/ObjectInspectorItem.js#168-170 and fix the tests that would fail because of this change.
I tried to look into the file mentioned. However, I was not able to locate the folder devtools-reps.
I have only the following folders in the packages directory.
- devtools-source-map
- devtools-splitter
- devtools-utils
- devtools-wasm-dwarf
Hello Karthik, sure you can give it a try.
The file was moved to https://searchfox.org/mozilla-central/rev/63fcc3f1a2cc73488d8986f4cf91fce2cd4b7564/devtools/client/shared/components/object-inspector/components/ObjectInspectorItem.js#123-125
|
|
Assignee | |
Comment 28•2 years ago
|
||
In Comment 8 and 9, two cases are given, which is the undesired behaviour? Also, it will be really helpful, if I get some testcases to test my code, for instance the code in the Screenflow.gif.
None of them seem to picture what's wanted.
Let me recap:
- Open the console
- Evaluate
[1,2,3]( - We now have
▶︎ Array(3) [ 1, 2, 3 ]printed in the console - Click on the arrow to expand the object
- We now have
▼ (3) […]
| 0: 1
| 1: 2
| 2: 3
| length: 3
| ▶︎ <prototype>: Array []
the purpose of this bug is to have
▼ (3) Array(3) [ 1, 2, 3 ]
| 0: 1
| 1: 2
| 2: 3
| length: 3
| ▶︎ <prototype>: Array []
instead (so the first line stays the same, we only expand the arrow)
It should limit some of the jumping we see.
|
|
Assignee | |
Comment 30•2 years ago
|
||
I have been able to achieve this result. Shall I submit my code for review???
sure! You can follow https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-write-a-patch and https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-submit-a-patch to do that
|
|
Assignee | |
Comment 32•2 years ago
|
||
|
|
Assignee | |
Comment 33•2 years ago
|
||
I just want to notify that I have submitted my patch. I hope I didn't do anything wrong in submitting my patch. If any issues kindly contact me.
|
||
Comment 34•1 year ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/22ea220f8502 Removed the code responsible for shortening to remove the jumping around of objects when expanding. r=nchevobbe.
|
||
Comment 35•1 year ago
|
||
| bugherder | ||
Description
•