1150163 - UITour infopanel should have the close button inline with the title

Status: RESOLVED DUPLICATE of bug 985175

(Firefox :: Tours, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screnshot of bug

See attached screenshot of the UITour infopanel. The blank line above the title is there just for the close button at the end of the line. We should place the close button at the end of the title, with enough margin between the title and close button so they are not right against each other.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of patch #1

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of patch #2

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 4

[Matthew N. [:MattN]]

The existing layout is according to mmaslaney's spec:
http://people.mozilla.org/~mmaslaney/onboarding/doorhanger-specs.png

Bug 985175 was going to polish some things. It seems like this comes down to preference so I would rather leave it as-is to avoid regressions during the rush to uplift.

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8586942 [details] [diff] [review]
Patch

Clearing review until UX strongly wants to change their mind on this one.

Comment 6

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Comment on attachment 8586939 [details]
Screenshot of patch #1

The current behavior is not on spec, because it only adds padding on the top (but not on the other sides, which makes it look unbalanced).

Jareds patch is closer to the spec in spirit (assuming that it looks similar on OS X), so let's go with it.

Comment 7

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Actually, would it still be possible to also add an equal amount of spacing on all other sides of the panel?

The current patch is definitely an improvement, so if those changes would keep us from uplifting we can skip them, but there's still room for improvement.

Comment 8

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Also, are you sure this doesn't regress anything?
For example, there are panels with icons in Fx dev edition that might look weird because of that...
http://cl.ly/image/2n013a2W132K

Comment 9

[Holly Habstritt Gaal [:Habber]]

(In reply to Philipp Sackl [:phlsa] from comment #8)
> Also, are you sure this doesn't regress anything?
> For example, there are panels with icons in Fx dev edition that might look
> weird because of that...
> http://cl.ly/image/2n013a2W132K

The door hangers used with the UI tours (the other collaborative web/chrome onboarding tours and in the Dev Edition screenshot you shared - anything that has used the tour API) are stylized to Mmaslaney's spec in comment 4. This may vary from standard in-product door hangers, but I'd suggest we don't cause any regression to any of the existing UI Tour door hangers unless we are clear about all the door hangers that will be impacted as a result. (Note: Reading List tour ended out not using the UITour API. The door hangers were actually implemented in-product, but we were staying consistent to the other onboarding tour door hanger styles)

If it is a goal to unify UITour door hangers with other in-product door hanger styles, even from this point moving forward, let's have a discussion before making changes.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Holly Habstritt Gaal [:Habber] from comment #9)
> (Note: Reading List tour ended out not using the UITour API. The
> door hangers were actually implemented in-product, but we were staying
> consistent to the other onboarding tour door hanger styles)

While it doesn't use the UITour API from the page content, we are still using the same UITour panels/doorhangers so any style changes for the readinglist tour would also affect other tours.

Comment 11

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Would it be an option to land this in Nightly and use QA to check for any regressions?

FWIW, the problem isn't new (at least on Windows). The first time I encountered it was with the search UI tour doorhangers.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Philipp Sackl [:phlsa] from comment #11)
> Would it be an option to land this in Nightly and use QA to check for any
> regressions?

Yeah that can work, although we'll want QA to put this higher up on their list as we'll want to uplift it pretty quickly if they don't find any regressions.