Closed Bug 1292048 Opened 3 years ago Closed 3 years ago

Move Geometry editor icon to make it less confusing

Categories

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

defect

Tracking

(firefox50+ verified, firefox51 verified)

VERIFIED FIXED
Firefox 51
Tracking Status
firefox50 + verified
firefox51 --- verified

People

(Reporter: gasolin, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, regression)

Attachments

(3 files)

Attached image UI spec
from UI spec in bug 1150496 , we'd like have a element picker at the right side of `Box Model` text
Helen, should we reuse the element picker icon at the top left side of devtools>inspector view, or would you like provide a new one?
Flags: needinfo?(hholmes)
after recheck the bug 1150496, I guess what you expect here is bug 1263768 (to show element's relationship to other elements on page) ?
I can see why this is confusing—that icon actually triggers bug 1139187, which is the geometry editor.

I think that that icon is poorly positioned, however. Documentation shows it appearing next to the value of "display" property, and I think we should keep it there instead of moving it.

I imagine that this would look like this: https://cl.ly/0O0n1e1G2a3K

(One thing to note: the geometry editor icon is on the right, not the left, of the value so that we can preserve the clean left-alignment on values.)
Flags: needinfo?(hholmes)
There's actually a geometry editor button already that is not displayed by default.
(In reply to Gabriel Luong [:gl] (ΦωΦ) from comment #4)
> Created attachment 8777868 [details]
> Screen Shot 2016-08-04 at 12.35.49 PM.png
> 
> There's actually a geometry editor button already that is not displayed by
> default.

I originally created designs not knowing where the geometry editor was.

Since we'd have to update documentation for this, should we consider moving it back next to "static"?
Flags: needinfo?(gl)
(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #5)
> (In reply to Gabriel Luong [:gl] (ΦωΦ) from comment #4)
> > Created attachment 8777868 [details]
> > Screen Shot 2016-08-04 at 12.35.49 PM.png
> > 
> > There's actually a geometry editor button already that is not displayed by
> > default.
> 
> I originally created designs not knowing where the geometry editor was.
> 
> Since we'd have to update documentation for this, should we consider moving
> it back next to "static"?

I think the positioning of the button is fine if we consider unifying moby's box model editor. We should ideally not hide the button and display it as disabled. Otherwise, I am ok with either keeping it in the same position or moing it back next to "static" if we are looking for a short term solution until unifying the box model editor. Ideally, we wouldn't move it back and forth too many times and cause too much user confusion and need for constant documentation updates.
Flags: needinfo?(gl)
The geometry editor icon was moved in Bug 1247729 which landed in Firefox 50. If we move it back next to the position, I think we should uplift it to Firefox 50, to avoid confusing users. Means we should take a decision asap to make a potential uplift easier.

The geometry editor will ultimately be merged with the box-model editor. But I feel like we're not yet sure where the final common icon will go or what it will look like. So there is a chance that merging the two editors will result in a UI change regardless of what we decide here. For this reason I am in favor of moving the icon back next to the "position" value, and uplifting this.

Helen, what do you think we should do here?
Flags: needinfo?(hholmes)
Priority: -- → P2
(In reply to Julian Descottes [:jdescottes] from comment #7)
> The geometry editor will ultimately be merged with the box-model editor. But
> I feel like we're not yet sure where the final common icon will go or what
> it will look like. So there is a chance that merging the two editors will
> result in a UI change regardless of what we decide here. For this reason I
> am in favor of moving the icon back next to the "position" value, and
> uplifting this.
> 
> Helen, what do you think we should do here?

I agree with you Julian, I think we should roll back this change. I'm sorry for letting this slip through, this was a mistake on my part.
Flags: needinfo?(hholmes) → needinfo?(gl)
Assignee: nobody → gl
Status: NEW → ASSIGNED
Flags: needinfo?(gl)
Summary: add element picker aside with Box Model → Move Geometry editor icon to make it less confusing
This is supposed to be uplifted to 50, so we need to land it ASAP.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9741d3087f83
Assignee: gl → jdescottes
Priority: P2 → P1
[Tracking Requested - why for this release]: Tracking to be sure it's not forgotten for 50 (per comment 10).
Comment on attachment 8789329 [details]
Bug 1292048 - move geometry editor icon back to its previous location;

https://reviewboard.mozilla.org/r/77612/#review75932
Attachment #8789329 - Flags: review?(gl) → review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/41ebaec6d7a7
move geometry editor icon back to its previous location;r=gl
Comment on attachment 8789329 [details]
Bug 1292048 - move geometry editor icon back to its previous location;

Sorry for the late approval request, I totally forgot about that bug until today, but it would be really nice if we could have it uplifted to FF50

Approval Request Comment
[Feature/regressing bug #]: Bug 1247729
[User impact if declined]: a devtool button moved to a confusing location by mistake compared to FF49  
[Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9741d3087f83&selectedJob=27153393, no new test for this as it only moves an existing node
[Risks and why]: small markup change, asking for an uplift to 50 here otherwise this button will move back and forth between FF49, 50 & 51 
[String/UUID change made/needed]: N/A

Patch applies fine on aurora, pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a78b7f34e27f
Attachment #8789329 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/41ebaec6d7a7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Comment on attachment 8789329 [details]
Bug 1292048 - move geometry editor icon back to its previous location;

Fixes a recent regression, Aurora50+
Attachment #8789329 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
I reproduced this issue  using Fx 51.0a1, build ID: 20160804030441, on Windows 10 x64.
I can confirm the Geometry Editor button has been moved under the box model pattern, in front of the position's value.

I verified on:
- Fx 50.0b1 (20160920155715)
- Fx 51.0a2 (20160919004008)
- Fx 52.0a1 (20160922030437)

Cheers!
Status: RESOLVED → VERIFIED
I think that there isn't any dev-doc-needed for this bug, since it reverts a change that was not documented. But I've updated the box-model docs for the move into the Computed view:

https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Reposition_elements_in_the_page
https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Examine_and_edit_the_box_model#The_Box_Model_view

Marking as dev-doc-complete, but let me know if this needs anything else.
Thanks a lot Will! I should have noted that the images and videos need updates.

I see one last thing. The first image at https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Reposition_elements_in_the_page still needs to be replaced.

Sebastian
Oh! Thanks Sebastian, I missed that one. It's done now.
Perfect, thank you!

Sebastian
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.