Closed Bug 1398104 Opened 7 years ago Closed 7 years ago

Have better style for "No cookies" and "No parameters"

Categories

(DevTools :: Netmonitor, enhancement, P3)

enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: nchevobbe, Assigned: abhinav.koppula, Mentored)

Details

Attachments

(3 files)

Attached image Mockup
If you have a network call that has no cookies or/and no parameters, we show a simple text in the panel saying "No cookies/parameters for this request" in a quite raw way.

We could style it in a nicer way (see attachment)
Hi Nicolas,
Have submitted a review-request for this issue on Mozreview

pasting terminal output-
changeset:  379428:f361fed2a706
summary:    Bug 1398104 - Improve styling of empty notice in "Cookies" and "Params" panel. r=nchevobbe
review:     https://reviewboard.mozilla.org/r/178034

review id:  bz://1398104/abhinavkoppula
review url: https://reviewboard.mozilla.org/r/178032
Comment on attachment 8906307 [details]
Bug 1398104 - Improve styling of empty notice in "Cookies" and "Params" panel.

https://reviewboard.mozilla.org/r/178032/#review183714

The patch is looking good ! Thanks for working on this
I don't really think we need the test though.
The "empty panel" behaviour is already tested, and here we test that we have the properties we set in the CSS rule.
It may happen that we have a change in the CSS that results in similar look, but would make the test fail, which
would only add work to the person changin this.
We do have something in CI to check visual regression, which would better test what we are trying to do here.
The idea is that the tool (called mozscreenshot), goes through some test cases, and take screenshot before and after the patch,
and if the output are different show you a UI where you can see the difference with and without the patch.

I need to check how we add test cases to that tool, and I'll keep you updated in this bug.
For now, I think you can remove the test from your patch and push to mozreview again. I'll then happily r+ it
Comment on attachment 8906307 [details]
Bug 1398104 - Improve styling of empty notice in "Cookies" and "Params" panel.

https://reviewboard.mozilla.org/r/178034/#review183850
Attachment #8906307 - Flags: review?(nchevobbe) → review-
Hi Nicolas,
Thanks for the review.
I have updated the mozreview-request and have removed the test.
Comment on attachment 8906307 [details]
Bug 1398104 - Improve styling of empty notice in "Cookies" and "Params" panel.

https://reviewboard.mozilla.org/r/178034/#review184262

Looking great, thanks ! Let's push to TRY
Attachment #8906307 - Flags: review?(nchevobbe) → review+
One small thing though.
When the cookies/parameters panel is very narrow, like in this attachment, the text is stuck to the left.
It would be nice if we have a little left and right padding.
Also, we could add `text-align: center` so it still looks nice even when the text is wrapped.

You can update your patch, push it to mozreview again. I already r+'d it so we'll just land it when TRY is over.
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/169c5daabde7
Improve styling of empty notice in "Cookies" and "Params" panel. r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/169c5daabde7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced this bug with 57.0a1 (2017-09-08) on Windows 7(64 Bit!).

This bug's fix is now verified on Latest Beta.

Build ID : 20171016185129
User Agent : Mozilla/5.0 (Windows NT 6.1; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20171025]
QA Whiteboard: [bugday-20171025] → [bugday-20171101]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.