Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SwapView element #183

Closed
cjolif opened this issue Jul 10, 2014 · 15 comments
Closed

SwapView element #183

cjolif opened this issue Jul 10, 2014 · 15 comments
Assignees
Milestone

Comments

@cjolif
Copy link
Contributor

cjolif commented Jul 10, 2014

We need to be able to swap from a view to another, possibly using ViewStack. There should be an indicator.

@cjolif cjolif added this to the 0.6.0 milestone Sep 10, 2014
@cjolif cjolif assigned edurocher and unassigned pruzand Sep 10, 2014
@cjolif cjolif modified the milestones: 0.5.0, 0.6.0 Sep 10, 2014
@edurocher
Copy link
Member

I am adding this comment to discuss various ideas/issues about SwapView.

  • The specification as I see it:
    • SwapView is a subclass of ViewStack, so we get all the ViewStack features: show/hide API, transitions, events.
    • In addition, thew SwapView supports a swipe gesture to swap to the next/previous view
    • The SwapView displays a view indicator (little dots at the bottom); it is questionable though if this could not go to ViewStack actually?
  • I have been experimenting an implementation that would (possibly) make it possible to reuse existing transitions to implement the swipe feedback. For example, with transition="slide", which basically moves the "in" view from 100% to 0% translation, the view would be translated to X% during the swipe gesture (X computed from the current finger position). It could be nice, for example, to see the view flipping or uncovering during swipe, instead of just slide. However, it seems a little too tricky and I have a hard time coming up with a general and simple implementation, so will probably drop that and just support a "slide" transition during swipe.
  • An issue I have come across is: do we support CSS class inheritance? To make ViewStack's transitions work in the SwapView subclass, the component needs to have the "d-view-stack" CSS class, although the SwapView will "naturally" have only the "d-swap-view" baseClass. Currently I am adding the "d-view-stack" class (in postRender), but not sure this is safe (there may be code that resets the class to the baseClass?).

@cjolif
Copy link
Contributor Author

cjolif commented Nov 20, 2014

The SwapView displays a view indicator (little dots at the bottom); it is questionable though if this could not go to ViewStack actually?

I think the idea was actually to make it a separated component that connects to a ViewStack. The reason for that was to allow any layout position without constraints.

@cjolif
Copy link
Contributor Author

cjolif commented Nov 20, 2014

An issue I have come across is: do we support CSS class inheritance? To make ViewStack's transitions work in the SwapView subclass, the component needs to have the "d-view-stack" CSS class, although the SwapView will "naturally" have only the "d-swap-view" baseClass. Currently I am adding the "d-view-stack" class (in postRender), but not sure this is safe (there may be code that resets the class to the baseClass?).

I don't think we support any form of inheritance here (@wkeese can confirm if needed).

Another possibility is having ViewStack CSS modified to do:

.d-view-stack, .d-swap-view {
}

instead of just

.d-view-stack {
}

Not sure which one is best.

@edurocher
Copy link
Member

@cjolif:

I think the idea was actually to make it a separated component that connects to a ViewStack.

OK makes sense.

@wkeese
Copy link
Member

wkeese commented Nov 20, 2014

IIRC we support having multiple baseClasses for a Widget. There might be some bugs though, but at least it worked in dijit.

I guess my comment is irrelevant if SwapView is "a separated component that connects to a ViewStack"... although I'm not sure what that means exactly. When rendering completes, is <d-view-stack> a child of <d-swap-view>?

@mythic2
Copy link

mythic2 commented Nov 21, 2014

I don't think we support any form of inheritance here

No, it does not appear so. However, the suggested solution does not provide any help for application developers who wants to override deliteful controls for what ever reasons.

@cjolif
Copy link
Contributor Author

cjolif commented Nov 21, 2014

I guess my comment is irrelevant if SwapView is "a separated component that connects to a ViewStack"... although I'm not sure what that means exactly. When rendering completes, is a child of ?

When talking about separate components I was talking about the page indicator not the SwapView.

@cjolif
Copy link
Contributor Author

cjolif commented Nov 21, 2014

No, it does not appear so. However, the suggested solution does not provide any help for application developers who wants to override deliteful controls for what ever reasons.

Right. In that case I guess the solution of @edurocher of adding a second base class in your subclass is doing the trick?

@mythic2
Copy link

mythic2 commented Nov 21, 2014

For my controls I solved it with definition like

baseClass: 'd-view-stack d-swap-view'

and it works well. If it is the right solution, that I have no idea :-)

@edurocher
Copy link
Member

That was my first idea too, then I looked at existing baseClass usages and I found things like this:

    domClass.toggle(this, this.baseClass + "-disabled", this.disabled);

(in https://github.com/ibm-js/deliteful/blob/master/StarRating.js#L108, but there are many other examples in deliteful) which made me think baseClass was supposed to be only one class (even though that code probably works if the widget's "real" base class is last in the list, but that seems a little fragile).

In any case it is useful to discuss this and clarify (and maybe also document?).

@wkeese
Copy link
Member

wkeese commented Nov 21, 2014

OK, nevermind, I guess baseClass just takes one class. Actually it is already documented that way:

/**
 * Root CSS class of the widget (ex: "d-text-box")
 * @member {string}
 * @protected
 */
baseClass: "",

@edurocher
Copy link
Member

Here is some preliminary work: edurocher@19c04db. Just the swap interaction with a very simple sample for now (and no tests yet).

@edurocher
Copy link
Member

And now a first shot at the ViewIndicator: edurocher@e535d81.

edurocher pushed a commit that referenced this issue Nov 28, 2014
Implements #183, but tests TBD.
@edurocher
Copy link
Member

Merged #398.
I will close this issue when the unit and functional tests are done.

@edurocher
Copy link
Member

Tests pushed (48fb578 and 89db608), closing this issue.

edurocher pushed a commit that referenced this issue Feb 6, 2015
Implements #183, but tests TBD.
wkeese pushed a commit to wkeese/deliteful that referenced this issue Mar 12, 2015
Implements ibm-js#183, but tests TBD.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants