Closed Bug 1428996 Opened 2 years ago Closed 2 years ago

Fix text wrap of boxmodel-editable

Categories

(DevTools :: Inspector: Computed, defect, P3)

defect

Tracking

(firefox60 verified)

VERIFIED FIXED
Firefox 60
Tracking Status
firefox60 --- verified

People

(Reporter: magicp.jp, Assigned: abhinav.koppula)

Details

(Keywords: good-first-bug)

Attachments

(2 files)

Steps to reproduce:
1. Launch Nightly
2. Go to https://bugzilla.mozilla.org
3. Open Inspector
4. Inspect #header-tools-menu
5. Switch sidebar tab to "Computed"

Actual results:
Text wrap and overlap with margin-top.

Expected results:
No wrap.
Thanks for filing and including the screenshot.

This looks like a CSS change that should be relatively simple to make.
I think we should make all of the labels inside this box-model view have the following property: white-space: nowrap;
This can be fixed in the following file: /devtools/client/themes/boxmodel.css
My hunch is that we should apply this too all .boxmodel-editable elements, but the person working on this bug should check that this doesn't have unwanted impacts.

I'll mark this as a good-first-bug because it shouldn't be hard to fix.
If you're new to Firefox and DevTools development, please read our docs to get started. And feel free to ask questions and claim this bug in comments here directly. I'd be happy to help you get this fixed.

Docs: http://docs.firefox-dev.tools/
Keywords: good-first-bug
Priority: -- → P3
Hi Patrick,
I've taken a shot at this issue.
Can you please review it?
Comment on attachment 8945829 [details]
Bug 1428996 - Fix text wrap of boxmodel-editable;

https://reviewboard.mozilla.org/r/215936/#review222054

Thank you! This seems to work fine. I think you can simplify the commit even more (see comment below).
Are you able to attach a new commit to fix this?

::: devtools/client/themes/boxmodel.css:311
(Diff revision 1)
> +  white-space: nowrap;
>  }
>  
>  .boxmodel-editable:hover {
>    border-bottom-color: hsl(0, 0%, 50%);
> +  white-space: nowrap;

Is this really needed? If this property is already defined on `.boxmodel-editable`, I don't think you need to define it too on `:hover`.
Attachment #8945829 - Flags: review?(pbrosset)
Comment on attachment 8945829 [details]
Bug 1428996 - Fix text wrap of boxmodel-editable;

https://reviewboard.mozilla.org/r/215936/#review222054

Yes, it works well even without defining in on :hover. I think that is redundant.
I've updated my patch.
Comment on attachment 8945829 [details]
Bug 1428996 - Fix text wrap of boxmodel-editable;

https://reviewboard.mozilla.org/r/215936/#review222060

Great. Thanks for being so quick.
Attachment #8945829 - Flags: review?(pbrosset) → review+
Hey Patrick,
There are some failures in TRY but I don't think they are related to this fix.
Can you check once?
Flags: needinfo?(pbrosset)
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
Yup, looks good to me.
Flags: needinfo?(pbrosset)
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c2881a440f7
Fix text wrap of boxmodel-editable; r=pbro
https://hg.mozilla.org/mozilla-central/rev/4c2881a440f7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Thanks Abhinav! I have verified in the latest Nightly build (20180201220225) on Win10, Ubuntu 17.10 and macOS 10.13.
On second thought, maybe we could uplift this?
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.