Closed Bug 1102483 Opened 10 years ago Closed 6 years ago

[Gallery] Refactor/Modularize view stack

Categories

(Firefox OS Graveyard :: Gaia::Gallery, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX
2.2 S2 (19dec)

People

(Reporter: justindarc, Unassigned)

References

Details

(Whiteboard: [ft:media] [2.2 target])

As part of the continued Gallery refactoring, the current `setView()` and `LAYOUT_MODE` pattern should be replaced with a component that is more flexible to accommodate the additional workflows/screens in v2.2. Ideally, we can encapsulate this as a web component where the child elements are considered views/screens/cards:

<gaia-view-stack id="view-stack">

  <!-- Home -->
  <gallery-home-view id="home-view">
  </gallery-home-view>  

  <!-- Grid -->
  <gallery-grid-view id="grid-view">
  </gallery-grid-view>

  <!-- Carousel -->
  <gallery-carousel-view id="carousel-view">
  </gallery-carousel-view>
</gaia-view-stack>

The <gaia-view-stack> component could expose some external API allowing us to programmatically push/pop views/states:

```
var viewStack = document.getElementById('view-stack');
viewStack.push('#grid-view', { param1: '...', param2: '...' });
...
viewStack.pop();
```

The <gaia-view-stack> component should also expose events that the application can observe to serve as "hooks" for setting up views:

```
viewStack.addEventListener('willpresentview', function(evt) {
  var view = evt.detail.view;
  var params = evt.detail.params;
  ...
  view.setParam1(params.param1);
  view.setParam2(params.param2);
});
```

NOTE: None of the above examples are set in stone. Just a brain dump of roughly what I had in mind.
Assignee: nobody → wilsonpage
Blocks: 2.2-gallery
Target Milestone: --- → 2.2 S2 (19dec)
Whiteboard: [ft:media] [2.2 target]
I'm not convinced that this should be a web component, nor that it needs to be a stack, but do agree that the existing code would benefit from some refactoring.

First of all, gallery doesn't currently have any screen transitions that require a stack. There is only one way to get to each screen and for any screen the back button always goes back to the same place. So we don't need a more general stack to take you back to where you came from.  The addition of albums could change that if we plan to have a different screens for each album. For memory efficiency, however, I suspect that we want to have only a single album screen populated with thumbnails at a time, however, so again, I think we may not need a stack. Am I missing something?

Also, the proposed gaia-view-stack component has no UI of its own and sounds like a very simple API. I'm not convinced that it is worth it.  If this was a component that displayed a title bar for its child views and managed the back button, that would seem like something worth having a component for. But that's not what we're doing here. So unless there is other API proposed that I'm missing, this doesn't seem like a component to me.  Transitions between screens is such a high-level thing that it really feels to me like application code, not component code, and I don't think we need to wrap it in a component.

setView() was originally simpler than it is now. You passed it the DOM element to show and it did that, hiding whatever was previously shown, and handling whatever special case code was needed to set up the new view and tear down the old view. It got more complicated when tablet support was added and that is what the LAYOUT_MODE stuff is. But if we're abandoning tablet support, then this can be simplified.

In particular having all the screen setup and teardown code inside setView is not good, and we should change that, perhaps by defining setup and teardown methods on the gallery-*-screen components that we're defining.

(Hmm... just had a related thought. One of the primary use cases of the existing setup/teardown code is that the select view and the main thumbnails view share the thumbnails. When we go into select mode we move the thumbnail container element from the main screen into the select screen. That way we don't have to create hundreds of duplicate thumbnails. So we'll probably want to make sure that the gallery-grid-view component is part of the regular content (not shadow dom) of the screen compoents that display grids so that we can do that kind of sharing and not get into shadow dom issues.)
Justin: thanks for starting this discussion, but you should have included me in the discussion, either with a needinfo here, or by talking to me before filing the bug.  Since we've got the bug already, I've put my thoughts above. 

Wilson: since you're the one signed up to work on this, I'd like to hear your thoughts about it. We can continue the technical discussion here (use needinfo for me, though or I won't see it), or we can set up a meeting to chat on vidyo.
Flags: needinfo?(wilsonpage)
Flags: needinfo?(jdarcangelo)
(In reply to David Flanagan [:djf] from comment #2)
> Justin: thanks for starting this discussion, but you should have included me
> in the discussion, either with a needinfo here, or by talking to me before
> filing the bug.  Since we've got the bug already, I've put my thoughts
> above. 
> 

David: You were not intentionally left out of this and the conversation around this started early yesterday morning when you were not around. Wilson wanted to get caught up on the Gallery developments to see where we're at and to find out what he can help with. The majority of the meeting was discussing the architecture we've started following in Bug 1093214. The topic of the view stack component came up as a result of us needing a better way to manage pushing/popping views to the screen. Gallery currently doesn't have any transitions between screens and makes it look unpolished compared to many other Gaia apps that already do have transitions. Since Wilson has been working on general-purpose Gaia web-components, we thought this would be a good place for him to start, especially since it is very clear that this is a common concern that many apps across all of Gaia may have. The idea behind componentizing it is to provide a clear definition for defining "screens" in an app while providing CSS hooks for specifying transition animations (if any) for each screen. We can also potentially add hooks for lazy loading assets required for each screen and there are many benefits to implementing this as a web component. Currently, every app in Gaia deals with this issue in an almost completely different manner, so having a web component to help deal with this could potentially help define a standard way for other Gaia apps to follow.
Flags: needinfo?(jdarcangelo)
JSBin of basic non-component implementation of a similar type of view state tracking:

http://jsbin.com/racaki
(In reply to Justin D'Arcangelo [:justindarc] from comment #3)
 The
> topic of the view stack component came up as a result of us needing a better
> way to manage pushing/popping views to the screen. 

I need to be convinced that we need to be able to pop. I assert (and I may be wrong) that we never need a stack, that every view has a static destination for the back button.

> Gallery currently doesn't
> have any transitions between screens and makes it look unpolished compared
> to many other Gaia apps that already do have transitions. 

UX has never asked us to implement transitions, and we shouldn't be planning to introduce them without asking first.

Since Wilson has
> been working on general-purpose Gaia web-components, we thought this would
> be a good place for him to start, especially since it is very clear that
> this is a common concern that many apps across all of Gaia may have. 

I don't think we have time to be developing components that are general purpose enough for other gaia apps. If Wilson is going to be working on Gallery, I'd prefer to have him focused on gallery-specific stuff. And if he's going to be working on general purpose components, he's got higher priorties than a view stack component. 

The
> idea behind componentizing it is to provide a clear definition for defining
> "screens" in an app while providing CSS hooks for specifying transition
> animations (if any) for each screen. We can also potentially add hooks for
> lazy loading assets required for each screen

What form would these hooks take? If you want to do a web component, I assume you're thinking custom HTML attributes, but that seems kind of awkward to me: you'd have to define a syntax and write a parser for it.  If you're just talking about an API for doing these things, then I don't see that this is a web componenty thing. 

 and there are many benefits to
> implementing this as a web component. 

You're going to have to be more specific than that! :-)

Currently, every app in Gaia deals
> with this issue in an almost completely different manner, so having a web
> component to help deal with this could potentially help define a standard
> way for other Gaia apps to follow.

I'm willing to be convinced, and perhaps fleshing out the details some more will convince me. But I just don't see how managing transitions between screens requires a container element and an additional level of web component nesting. It seems to me that this is pure controller level stuff and should have nothing to do with views.

And, to reiterate: solving this problem in a general way for the benefit of other Gaia apps is out of scope. Gallery's needs are relatively simple (currently no stack or transitions) and I'd like to keep the refactored solution simple too. If creating a gallery-specific view stack component is the best way to do this, please flesh out the details. Otherwise, let's go with a simple non-component module.
I'm confident that the use of a component here is a good move. I've been staying in touch with what the Polymer team is up to over at Google, and am especially impressed with there work in pushing web-components as the basis of a 'web-sdk'. This is Chrome Summit talk [1], for me, really drove home the power of 'composition' in modern web-component based app development.

Gallery already has it's views declared in markup. If each one of these views were able to define a 'route' via an attribute, then a parent component can take full control of showing and hiding views when routes and matched/unmatched.

I'm confident a 'compositional' approach will fit well into gallery's existing architecture, be a good basis on which to build new features and greatly reduce code size/complexity.

I've thrown together a rough proof of concept called <gaia-pages> [2]. Like good old traditional web-development, the URL is the source of truth. Routes are matched and URL parameters passed to corresponding pages to update content. The user of the component can define their transitions (might be sensible to have some defaults though). It's also RTL ready. We're using hash URLs (not History API) as we're a serverless single-page app. 

[1] https://www.youtube.com/watch?v=kV0hgdMpH28
[2] http://wilsonpage.github.io/gaia-pages/
Flags: needinfo?(wilsonpage)
Assignee: wilsonpage → nobody
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.