Watch expressions should wrap to show result
Categories
(DevTools :: Debugger, defect, P3)
Tracking
(firefox57 affected)
Tracking | Status | |
---|---|---|
firefox57 | --- | affected |
People
(Reporter: trebitzki, Assigned: rich)
References
(Blocks 2 open bugs)
Details
(Keywords: good-first-bug)
Attachments
(5 files, 1 obsolete file)
Reporter | ||
Updated•11 years ago
|
Updated•11 years ago
|
Updated•11 years ago
|
Updated•11 years ago
|
Updated•10 years ago
|
Updated•10 years ago
|
Comment 1•7 years ago
|
||
Updated•6 years ago
|
Updated•5 years ago
|
Comment 2•5 years ago
|
||
I'll gladly pick this up.
Updated•5 years ago
|
Comment 3•5 years ago
|
||
I've used the CSS editor to create a working demo of what this feature would look like.
When this bug was originally filed, the proposed solution is that watch expressions "should be wrapped to as many lines as necessary to show the whole result."
While this is desirable in theory, it creates UI problems in practice. As the panel narrows, the wrapping of the expression would force the panel to lengthen vertically.
In the video, you can it would look like for string expressions if overflow-wrap
was set to break-word
. The width of the string will decrease until it reaches the width of the narrowest word in the string.
Comment 4•5 years ago
|
||
the width of the narrowest word in the string.
I meant widest word in the string, not narrowest.
Comment 5•5 years ago
|
||
The situation worsens if overflow-wrap
is set to anywhere
, as the expression width will decrease all the way to the width of a single character, potentially making the panel even taller than it was before.
Even if we use break-word
for strings (as described in my previous comment), this is the solution we would have to implement for numbers, as they can't break into words.
Comment 6•5 years ago
|
||
As an alternative solution, I would like to suggest that we seek parity with Chrome, and avoid wrapping watch expressions altogether.
Under Chrome's implementation, watch expressions are not wrapped when the right sidebar is resized. To see the whole expression when the panel is narrowed, users have the option to hover over the expression and view the entire expression as a tooltip.
Comment 7•5 years ago
|
||
Our current implementation is very close to Chrome's. The only difference is that our tooltip on hover displays the property name, not the property value.
To resolve this bug, I would like to fix the tooltip to display the property value. I spoke with David Walsh during the debugger community call today, and he also agreed this would be a worthwhile solution.
Comment 8•5 years ago
|
||
This patch addresses issues discussed on bug 915854. It modifies the tooltip of watch expressions in the watch expressions panel to display information related to the value of that watch expression, rather than the key. It also appends ellipses to the end of watch expressions values that have been cut off by the width of the watch expressions panel. Furthermore, it removes a secondary bug that was forcing symbol watch expressions to wrap in the watch expressions pane.
Comment 9•5 years ago
|
||
One note, in your last video the value of the expression was overlayed by the x to remove the expression. So, that needs to be fixed if not already done. The simplest solution is probably to add a display: flex;
to the container of the expression.
Sebastian
Updated•5 years ago
|
Comment 10•5 years ago
|
||
(In reply to Sebastian Zartner [:sebo] from comment #9)
One note, in your last video the value of the expression was overlayed by the x to remove the expression. So, that needs to be fixed if not already done. The simplest solution is probably to add a
display: flex;
to the container of the expression.Sebastian
Hi Sebastian!
I just built the tip of the default
branch, and found that the UI bug you mentioned is still present. I'll fix it in my feature build and add it to my WIP patch. Thank you for the feedback!
Comment 11•5 years ago
|
||
Hi Kevin. I'm just checking if you are still interested in working on this bug. There hasn't been any activity here or on the patch in phabricator for some time. If you just need more time, that's fine. If you, however, are not planning on working on this bug anymore, let us know so it can be made available for others to pick up.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 12•5 years ago
|
||
Can I work on this? It seems that Kevin is not interested in it anymore.
Comment 13•5 years ago
|
||
It's yours. Thanks a lot for your help! Feel free to ask questions in Slack or here if you need to ramp up.
Comment 14•5 years ago
|
||
Hi petcuandrei, I'm just checking in to see how you're doing. Do you need any help with the work on this? Do you still intend to work on this bug?
Comment 15•5 years ago
|
||
I'm going to unassign this bug for now since petcuandrei did not answer. Maybe someone else wants to pick up from here.
Comment 16•5 years ago
|
||
Sorry for the late repply. I cannot work on this :(
Comment 17•5 years ago
|
||
Hello!
I am new to contributing to the Firefox DevTools and would like to work on this.
Assignee | ||
Comment 18•5 years ago
|
||
Gah! beat me to the punch! @tehlordvortex - if you decide to work on another bug id love to take this one. also looking for my first bug to work on, and I know Kevin, who did some of the work previously on this bug.
Comment 19•5 years ago
|
||
@rich sure, no problem! I'll pick up another bug ;)
Assignee | ||
Comment 20•5 years ago
|
||
Oh thanks - didn't expect that! I appreciate your flexibility Victor! Harold or Patrick - can i claim this as my first bug?
Comment 21•5 years ago
|
||
Thanks :rich. Feel free to ask questions here or in our team chat's #debugger channel.
Assignee | ||
Comment 22•5 years ago
|
||
Thanks! Will do!
Assignee | ||
Comment 23•5 years ago
|
||
:digitarald and :jlast I've put together a quick POC of my proposed solution. I can also commit my changes for discussion (I may need help with the workflow for this first time through for getting it up to phabricator). My solution provides a native title tooltip for primitive data types rendered using the ObjectInspector
component. I've also noticed that the original bug's solve for trimming the text in a long string does not currently work down in the scopes pane, so that may be worth looking into as well to ensure that the end functionality is the same for each place that uses the ObjectInspector
component.
If this does align with desired functionality I'd like to discuss in more detail some of the considerations we need to make for extensibility - may be worth having that conversation in slack and/or a google doc due to the number of files involved.
POC video:
https://www.loom.com/share/2b4477a61849475eae8debb803704285
Assignee | ||
Comment 24•5 years ago
|
||
:digitarald and :jlast I have a suggestion that we mark this bug as complete, since Kevin's work before me addressed truncating long values and exposing the 'remove watch expression' x button.
Kevin made a recommendation that we also include some form of more helpful context tooltips for those longer values, and since most of the work I've been doing is on these tooltip titles, and it seems too unrelated to this particular bug to keep it housed here, I've created another bug to address the larger issue at hand. https://bugzilla.mozilla.org/show_bug.cgi?id=1631545. Please advise!
Comment 25•4 years ago
|
||
Sorry for the delay, :rich! Closing this makes sense, thanks a lot for looking it – looking forward to see your work land for bug 1631545!
Description
•