Closed Bug 1497181 Opened 1 year ago Closed 1 year ago

Handle text node flex items

Categories

(DevTools :: Inspector, enhancement, P2)

enhancement

Tracking

(firefox64 fixed)

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: pbro, Assigned: pbro)

References

Details

Attachments

(1 file)

For now if a flex item is in fact a text node (wrapped in an anonymous element), it just is not displayed in the flexbox inspector as an item, nor can you see its sizing information.

We should handle them.
Here's one test case that shows a few problems:

data:text/html,<div style="display:flex">this is just a text node that will be wrapped in an anonymous flex item element</div>

If you select the <div> in the inspector, then the sidebar does show that there is 1 flex item, but it shows it as {}. Instead it should display it like we do for TextNode Reps (with a preview of the text content).

Second issue is if you click on that item in the list, then the accordion tells you there's no flex item or container here.

Now, second test case: on http://flexbox.buildwithreact.com/, select one of the numbered squares.
Same issue with showing {}. 
But then, we're going to have another issue: the logic we have for flex items is that when you select one in the list, it becomes selected in the inspector too. However on that example, the text node is short enough to be "inlined" in its parent node. What that means is it appears like this in the inspector: 

<div style="align-items:center;display:flex;height:50px;justify-content:center;width:50px;background-color:rgb(220, 80, 80);" data-reactid=".0.$/=10=20.0.$/=11.0.0.$=1$2.0">3</div>

Instead of:

<div style="align-items:center;display:flex;height:50px;justify-content:center;width:50px;background-color:rgb(220, 80, 80);" data-reactid=".0.$/=10=20.0.$/=11.0.0.$=1$2.0">
  3
</div>

So that means the text node itself (3) can't be selected in the inspector. It does not exist as a separate node there (however it does in the DOM).
Duplicate of this bug: 1496716
Assignee: nobody → gl
Status: NEW → ASSIGNED
This is now better than it used to be when I filed the bug. The #textNode does appear in the list of items. Clicking it still leads to an empty accordion though.
While looking at this I realized that the fix will be very similar to the one I'm making over in bug 1499322. Gabriel: are you ok with me stealing this bug (well, marking it as a dupe of bug 1499322 and doing the fix over there instead). I think this will save us a conflicting merge.
Flags: needinfo?(gl)
(In reply to Patrick Brosset <:pbro> from comment #3)
> This is now better than it used to be when I filed the bug. The #textNode
> does appear in the list of items. Clicking it still leads to an empty
> accordion though.
> While looking at this I realized that the fix will be very similar to the
> one I'm making over in bug 1499322. Gabriel: are you ok with me stealing
> this bug (well, marking it as a dupe of bug 1499322 and doing the fix over
> there instead). I think this will save us a conflicting merge.

Sure that's fine.
Flags: needinfo?(gl)
Assignee: gl → nobody
Status: ASSIGNED → NEW
Thanks. Actually, I'll still use this bug and make it depend on bug 1499322.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Depends on: 1499322
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b786be7eb3b5
Display sizing info for text nodes too; r=gl
https://hg.mozilla.org/mozilla-central/rev/b786be7eb3b5
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.