Closed Bug 1284461 Opened 4 years ago Closed 3 years ago

Update variable height behaviour for HTMLTooltip

Categories

(DevTools :: Shared Components, defect, P1)

defect

Tracking

(firefox50 fixed)

RESOLVED FIXED
Firefox 50
Iteration:
50.3 - Jul 18
Tracking Status
firefox50 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

(Whiteboard: [devtools-html])

Attachments

(1 file)

When migrating the EventDetails tooltip to use the HTMLTooltip, I changed the HTMLTooltip behaviour in order to make the preferred height specified by the user act a max-height.

This is useful when the height of the tooltip might vary and you want to allow the tooltip content to expand dynamically as much as possible.

However when you are using fixed height tooltips (such as the ruleview editors and the MDN docs previews) it is easier if the height of the tooltip is set.

To change this behaviour we have two options:

- using {height: Infinity} makes the preferred height act as max-height, any other value acts as a static height
- add an additional parameter to the options object expected by HTMLTooltip::setContent {variableHeight:true/false}
Whiteboard: [devtools-html] [triage]
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment on attachment 8767956 [details]
Bug 1284461 - HTMLTooltip: use variable height only if height==Infinity;

Switching to feedback for now to start the discussion around the best solution here.
Attachment #8767956 - Flags: review?(bgrinstead) → feedback?(bgrinstead)
https://reviewboard.mozilla.org/r/62358/#review59118

I like the height: Infinity solution since it avoids adding a new option. So I guess if a tooltip wants to do 'min-height' it should be done with CSS on the container?

::: devtools/client/themes/tooltips.css:138
(Diff revision 1)
>  }
>  
> +/* Tooltip : flexible height styles */
> +
> +.tooltip-flexible-height .tooltip-panel {
> +  /* In flexible  mode the tooltip panel should only grow according to its content. */

Nit: extra space

::: devtools/client/themes/tooltips.css:143
(Diff revision 1)
> +  /* In flexible  mode the tooltip panel should only grow according to its content. */
> +  flex-grow: 0;
> +}
> +
> +.tooltip-flexible-height .tooltip-filler {
> +  /* In flexible  mode the filler should grow as much as possible. */

Ditto
Consider that an r+, if you are happy with the patch also
Component: Developer Tools: Framework → Developer Tools: Shared Components
Comment on attachment 8767956 [details]
Bug 1284461 - HTMLTooltip: use variable height only if height==Infinity;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62358/diff/1-2/
Attachment #8767956 - Flags: feedback?(bgrinstead) → review?(bgrinstead)
Comment on attachment 8767956 [details]
Bug 1284461 - HTMLTooltip: use variable height only if height==Infinity;

https://reviewboard.mozilla.org/r/62358/#review59152
Attachment #8767956 - Flags: review?(bgrinstead) → review+
Thanks for the review! Happy with this approach too, so let's go ahead. Fixed the extra spaces and rebased.

Try is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=05c8a2ef35bda353cf70f9fc0c55a792d7361253
Iteration: --- → 50.3 - Jul 18
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [devtools-html] [triage] → [devtools-html]
This bug will not have any visible impact, setting qe-verify-.
Blocking on Bug 1267414.
Depends on: 1267414
Flags: qe-verify? → qe-verify-
Comment on attachment 8767956 [details]
Bug 1284461 - HTMLTooltip: use variable height only if height==Infinity;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62358/diff/2-3/
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/489d075c7676
HTMLTooltip: use variable height only if height==Infinity;r=bgrins
https://hg.mozilla.org/mozilla-central/rev/489d075c7676
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.