Expanding objects makes them jump around
Categories
(DevTools :: Shared Components, defect, P3)
Tracking
(firefox88 fixed)
| Tracking | Status | |
|---|---|---|
| firefox88 | --- | fixed |
People
(Reporter: Harald, Assigned: karthiksundar30092002)
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•1 year ago
|
||
Nicolas, is there some prior arguments on why expanded objects change shape?
|
||
Comment 2•1 year ago
|
||
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•1 year 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•1 year 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•1 year 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•1 year 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•1 year 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.
|
|
||
Comment 14•1 year ago
|
||
Thanks for the heads-up Samarjeet, I unassigned the bug.
|
|
||
Comment 16•1 year ago
|
||
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•1 year ago
|
||
Thanks for the info Nick!
|
|
||
Comment 18•1 year 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.
|
|
||
Comment 19•1 year ago
|
||
Hello Mary,
Could you try to do yarn install before? We need to pull those dependencies from npm :)
|
|
||
Comment 20•1 year 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?
|
|
||
Comment 21•1 year ago
|
||
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•1 year 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.
|
|
||
Comment 23•11 months ago
|
||
can you try to run moz-phab install-certificate again?
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 months ago
|
||
Can I take this bug??, If it is still open
|
|
Assignee | |
Comment 26•2 months 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
|
|
||
Comment 27•2 months ago
|
||
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 months 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.
|
|
||
Comment 29•2 months ago
|
||
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 months ago
|
||
I have been able to achieve this result. Shall I submit my code for review???
|
|
||
Comment 31•2 months ago
|
||
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 months ago
|
||
|
|
Assignee | |
Comment 33•2 months 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.
|
|
||
Updated•2 months ago
|
|
||
Comment 34•1 month 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 month ago
|
||
| bugherder | ||
Description
•