903002 - Display properly the widget with panel attached, in Australis

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Matteo Ferretti [:zer0] [:matteo]]

Now that bug 880458 is fixed, we can modify our Widget SDK code in order to display the panel attached even if the widget is overflowed or in menu panel. The proper behavior is hide the overflow/menu panel, and display the widget's panel using the chevron (when is overflowed) or the menu button (when is in the menu panel) as anchor.

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

I already have a patch for this, I'm working on the unit tests.
Notice, the bug 880458 is not marked as fixed, but is fixed in UX branch.

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

Pointer to Github pull-request

Comment 3

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 792846 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

I forget actually to attach the PR with the work in progress

Comment 4

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

Considering we don't want panels floating out of nowhere, marking this as a P3. Matteo, is there any update on this?

Comment 5

[Matteo Ferretti [:zer0] [:matteo]]

I was in vacation, is done but still needs the unit test: I will fix it before the end of this week.

Comment 6

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

Pointer to Github pull-request

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 792846 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

I made some suggestions on how to make code added more readable to others, feel free to disregard.

Comment 8

[Matteo Ferretti [:zer0] [:matteo]]

Irakli, I asked another review because I was notified that bug 906634 was fixed, and that simplify a lot the patch. In addition, I'm not sure what you meant with this comment: https://github.com/mozilla/addon-sdk/pull/1172#discussion_r6978502
Additionally, I found a bit confusing too the comment you suggested in https://github.com/mozilla/addon-sdk/pull/1172#discussion_r6977622, and I tried to improved with the help of other devtools and fx-team members; hope it will be more clear for you too.

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 792846 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

Thanks, this looks good to me!

Comment 10

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 792846 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

Due the comment of Gijs about using the proper CustomizableUI method instead of accessing directly the `customizableui-anchorid`, I had to change the again the code and the comments.

Comment 11

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 792846 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1172

Looks good, thanks!

Comment 12

[Justin Dolske [:Dolske]]

Is this fixed now?

Comment 13

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Justin Dolske [:Dolske] from comment #12)
> Is this fixed now?

It does not seem to be landed, so no. ZER0 is there any reason why this isn't landed yet ?
If not, could you please land it.

Comment 14

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/232171d6322fe7152b73eb994de90459d031ef9e
Bug 903002 - Display properly the widget with panel attached, in Australis

https://github.com/mozilla/addon-sdk/commit/6a8d60d6a9998aed0412226e35efedc0eb867853
Merge pull request #1172 from ZER0/australis-widget/903002

Bug 903002 - Display properly the widget with panel attached, in Australis r=@gozala

Comment 15

[Matteo Ferretti [:zer0] [:matteo]]

No, I thought was landed, just merged now.

Comment 16

[:Gijs (he/him)]

Isn't this fixed now?

Comment 17

[Matteo Ferretti [:zer0] [:matteo]]

It is! I was waiting after the merge on our repo that was merged with Australis and it slipped my mind.