Bug 151235 – Web Inspector: Type popover should shrink to its content size

• Component: Web Inspector
• Status: NEW

[Nikita Vasilyev]

Created attachment 265451 [details]
[Image] Bug

See the attached screenshot. There is no reason for the type popover to be that tall.

[Radar WebKit Bug Importer]

<rdar://problem/23527020>

[Nikita Vasilyev]

Created attachment 265453 [details]
[Image] Debugger popovers

For debugger, popovers are nice and small (see the attached image). Type popovers should be the same.r

[Daniel Strokis]

Created attachment 273896 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 273896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273896&action=review

> Source/WebInspectorUI/ChangeLog:8
> +        Popovers in the source code editor that were cuased by hovering over a symbol were unnecessarily large due to min-width and min-height rules. Removed those style rules.

I think this was added to have better basic support for Object/Class popovers which dynamically fill in content. What happens when you hover an Object/Element/Node type bubble that has a list of properties?

[Joseph Pecoraro]

(In reply to comment #4)
> Comment on attachment 273896 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=273896&action=review
> 
> > Source/WebInspectorUI/ChangeLog:8
> > +        Popovers in the source code editor that were cuased by hovering over a symbol were unnecessarily large due to min-width and min-height rules. Removed those style rules.
> 
> I think this was added to have better basic support for Object/Class
> popovers which dynamically fill in content. What happens when you hover an
> Object/Element/Node type bubble that has a list of properties?

If this is the case, perhaps a better solution would be to set some default min size for "Object" popovers and resize the popover to better fit when content changes dynamically.

[Daniel Strokis]

(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 273896 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=273896&action=review
> > 
> > > Source/WebInspectorUI/ChangeLog:8
> > > +        Popovers in the source code editor that were cuased by hovering over a symbol were unnecessarily large due to min-width and min-height rules. Removed those style rules.
> > 
> > I think this was added to have better basic support for Object/Class
> > popovers which dynamically fill in content. What happens when you hover an
> > Object/Element/Node type bubble that has a list of properties?
> 
> If this is the case, perhaps a better solution would be to set some default
> min size for "Object" popovers and resize the popover to better fit when
> content changes dynamically.

AFAICT from testing, popovers dynamically resize for Objects, but not for Nodes. I think having a class for Nodes that retains the min-width and min-height rules could be a viable solution.

[Daniel Strokis]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Comment on attachment 273896 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=273896&action=review
> > > 
> > > > Source/WebInspectorUI/ChangeLog:8
> > > > +        Popovers in the source code editor that were cuased by hovering over a symbol were unnecessarily large due to min-width and min-height rules. Removed those style rules.
> > > 
> > > I think this was added to have better basic support for Object/Class
> > > popovers which dynamically fill in content. What happens when you hover an
> > > Object/Element/Node type bubble that has a list of properties?
> > 
> > If this is the case, perhaps a better solution would be to set some default
> > min size for "Object" popovers and resize the popover to better fit when
> > content changes dynamically.
> 
> AFAICT from testing, popovers dynamically resize for Objects, but not for
> Nodes. I think having a class for Nodes that retains the min-width and
> min-height rules could be a viable solution.

The popover for a Node does show all of the data, but it feels cramped.

[Blaze Burg]

Comment on attachment 273896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273896&action=review

Thanks for submitting a patch. Please include screenshots of the various cases Joe mentioned with this patch applied so we can move forward.

>>>>> Source/WebInspectorUI/ChangeLog:8
>>>>> +        Popovers in the source code editor that were cuased by hovering over a symbol were unnecessarily large due to min-width and min-height rules. Removed those style rules.
>>>> 
>>>> I think this was added to have better basic support for Object/Class popovers which dynamically fill in content. What happens when you hover an Object/Element/Node type bubble that has a list of properties?
>>> 
>>> If this is the case, perhaps a better solution would be to set some default min size for "Object" popovers and resize the popover to better fit when content changes dynamically.
>> 
>> AFAICT from testing, popovers dynamically resize for Objects, but not for Nodes. I think having a class for Nodes that retains the min-width and min-height rules could be a viable solution.
> 
> The popover for a Node does show all of the data, but it feels cramped.

Typo: 'caused'

[Blaze Burg]

Comment on attachment 273896 [details]
Patch

Clearing r? since it seems that you were going to improve handling of the Node popover case. (FYI, in the future, adding screenshots of relevant cases to the bug can make reviewing go a lot faster.)