Closed Bug 1161089 Opened 5 years ago Closed 5 years ago

Handling an autotheme WebActivity while a theme is selected will wrongly add the palette to this theme

Categories

(Firefox OS Graveyard :: Gaia::Theme Editor, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: etienne, Assigned: etienne)

References

Details

Attachments

(1 file)

37 bytes, text/x-github-pull-request
julienw
: review+
Details | Review
There's a bit of Navigation cleanup to do to fix this properly.

+ Add a Navigation.popToRoot() to the Navigation
+ Call Navigation.popToRoot() in the ActivityHandler when we start
+ In Navigation#pop, dispatch a `willhide` event on the panel before poping it
+ in Detail, remove the `AutoTheme:palette` event listener when we get the `willhide` CustomEvent
I'll take this one.
Assignee: nobody → etienne
Priority: -- → P1
Blocks: 1161098
Attached file Gaia PR
Hope you'll like it :)
Attachment #8601592 - Flags: review?(felash)
Comment on attachment 8601592 [details] [review]
Gaia PR

Some nits but especially popToRoot is not working properly :)
Also I don't know what the CSS change is for.
Attachment #8601592 - Flags: review?(felash)
Comment on attachment 8601592 [details] [review]
Gaia PR

Thanks for the review! Updated!
Attachment #8601592 - Flags: review?(felash)
Comment on attachment 8601592 [details] [review]
Gaia PR

r=me, let's land this.

For posterity, here is my alternate implementation for popToRoot:

  popToRoot: function() {
    var currentLength = stack.length;
    var pops = [];
    for (var i = 0; i < currentLength - 1; i++) {
      pops.push(this.pop());
    }
    return Promise.all(pops).then(() => {});
  }

If we ever want to go that path, we'll need to take care we correctly handle possible missing transition events.
Attachment #8601592 - Flags: review?(felash) → review+
Whoops, this landed a while ago
https://github.com/fxos/studio/commit/bc5729d15c990ec773aacf8534387c423f85e170
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.