Closed
Bug 1256422
Opened 9 years ago
Closed 9 years ago
Move the breadcrumbs into a bottom toolbar
Categories
(DevTools :: Inspector, defect, P2)
DevTools
Inspector
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: gl, Assigned: gl)
References
(Depends on 1 open bug)
Details
(Keywords: dev-doc-complete, Whiteboard: [btpp-fix-later])
Attachments
(3 files)
(deleted),
patch
|
bgrins
:
review+
gl
:
ui-review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details |
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8730352 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 2•9 years ago
|
||
Comment 3•9 years ago
|
||
There has been some discussions to remove it altogether. CC'ing Helen.
Whiteboard: [btpp-fix-later]
Comment 4•9 years ago
|
||
Comment on attachment 8730352 [details] [diff] [review]
1256422.patch
Review of attachment 8730352 [details] [diff] [review]:
-----------------------------------------------------------------
::: devtools/client/themes/inspector.css
@@ +13,5 @@
> }
> #inspector-searchlabel {
> overflow: hidden;
> }
> +#inspector-breadcrumbs-toolbar {
My 2 cents: it looks better if we override the background color on inspector-breadcrumbs-toolbar to be the normal body background color. Especially on the dark theme. What do you think Helen? You could apply the patches locally or possibly check out the screenshots on these try pushes if they work out:
* Current patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb0e6f05cbd1
* With proposed change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e5052318720
@@ +16,5 @@
> }
> +#inspector-breadcrumbs-toolbar {
> + padding: 0px;
> + border-bottom-width: 0px;
> + border-top-width: 1px;
I wish this didn't need the border (with my suggested change above applied), but it would look funny if the crumbs are just floating around. Probably that would be possible if we made the crumbs absolutely positioned at the bottom of the viewport, but not needed for this bug if we want to land something first without a big redesign of the crumbs feature (which I think we do)
Attachment #8730352 -
Flags: ui-review?(hholmes)
Attachment #8730352 -
Flags: review?(bgrinstead)
Attachment #8730352 -
Flags: review+
Comment 5•9 years ago
|
||
Alternate patch with the idea from comment 4 in case you want to apply them locally
Comment 6•9 years ago
|
||
Regarding comment 4 and the :root / dark theme stuff I like this pattern: https://dxr.mozilla.org/mozilla-central/source/devtools/client/sourceeditor/codemirror/mozilla.css#5
Assignee | ||
Updated•9 years ago
|
Attachment #8730352 -
Flags: ui-review?(hholmes) → ui-review+
Comment 8•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 9•9 years ago
|
||
So I messed around with the breadcrumbs after reading through Bug 1262668, and I agree with both of Brian's points in Comment 4—it'd be nice to keep the background-color the background-color of the Inspector, and I think the border shouldn't be the same as the splitter—it causes some confusion.
So, I'd recommend the background be var(--theme-body-background), and that we introduce a new variable for #fafafa (border for non-splitters).
In my mockup I'm also using different colors than we currently have in the breadcrumbs and inspector, but those I think should be addressed in bug 1246313.
Updated•9 years ago
|
Keywords: dev-doc-needed
Comment 10•9 years ago
|
||
I've updated: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Examine_and_edit_HTML#HTML_breadcrumbs
I've also updated the "UI tour" page, so it's consistent: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/UI_Tour.
Marking as dev-doc-complete, but let me know if I need anything else, please.
Keywords: dev-doc-needed → dev-doc-complete
Updated•9 years ago
|
Comment 11•8 years ago
|
||
Note that people are complaining on Stack Overflow about the breadcrumb bar being moved to the bottom.
Unfortunately the bug doesn't give any reason why this was done. At the moment it looks like we just lost vertical space by this change.
Gabriel, could you please explain it?
Sebastian
Flags: needinfo?(gl)
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Sebastian Zartner [:sebo] from comment #11)
> Note that people are complaining on Stack Overflow about the breadcrumb bar
> being moved to the bottom.
> Unfortunately the bug doesn't give any reason why this was done. At the
> moment it looks like we just lost vertical space by this change.
>
> Gabriel, could you please explain it?
>
> Sebastian
There are a couple of reasons here that made us move it to the bottom. We wanted to repurpose the space that the breadcrumbs occupied - to make new and existing features more visible. The start of this was adding new buttons to that space - the create new node in this case. There were additional work to add some of the toolbox buttons to that space as well. There were discussions to remove the breadcrumbs altogether, but we ultimately felt we should move it to the bottom to make it less obtrusive. We will continue to enhance that newly freed top space in the inspector in Q4.
Flags: needinfo?(gl)
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•