[v-play] add venue description implementation #65
Conversation
Both, the Sailfish SDK and the v-play SDK, offer an IconButton implementation. However, both implementations differ, so that we need to wrap around that. The function 'iconBy(type)' returns the framework specific implementation. Ref #48
The design is far away from being perfect on v-play, but it is a good starting point for further development. Ref #48
Therefore we connect the Sailfish's clicked(int index) signal with the selected(int index) signal of v-play's SimpleRow and we pass through Sailfish's PageStack to v-play's NavigationStack. Drawback: This commit introduces some type error, which we have to address in the future. Ref #48
| import "." as BVApp | ||
|
|
||
| IconButton { | ||
| function iconBy(type) { |
There was a problem hiding this comment.
That IconButton abstraction is amazing, nice! I have also already thought about this a bit (when thinking about the Theme), and I head exactly something like this in mind ;). Of course, I haven't implemented it / tried it out. Very nice work!
One idea: What would you think about moving this iconBy() function to the Theme class? Maybe we could do sth. like
Theme
{
...
function icon(type, scale)
...
}
I see the following advantages with that approach:
- We don't have provide a wrapper for the
IconButtonimplementation. - We can use this function outside of the
IconButtonuse case, when we need icons with native look and feel.
There was a problem hiding this comment.
Thanks. I think it's a good idea to move it into the Themes, but shall the function then return an array with [iconString, scalingFactor] or what did you think of?
There was a problem hiding this comment.
I'd suggest just to move the functions iconBy(type) into the according Themes and set the scaling factor from the outside, as it is now already.
There was a problem hiding this comment.
My idea was that it actually returns the icon type that is used on the corresponding platform: A string on v-play and a (scaled) icon on Sailfish, which both can be directly assigned to the icon object of the IconButton. But I just saw that we then need to export the pressed property as a parameter of this function, as well, right? Does v-play have sth. like this? I assume yes, and I assume it is named differently. ;)
There was a problem hiding this comment.
They have something how the icon should look like AFTER it was pressed (what we actually need for the upcoming favourites feature), but not for the pressed state itself.
I am not quite sure, what you want to achieve. So I'd suggest to move the function as it is, so it is at the right place and to postpone further improvements. I have to think about the whole VenueDetails-design anyways.
Ok?
There was a problem hiding this comment.
Oh, I was wrong, I think pressed is called selected in v-play...
There was a problem hiding this comment.
OK. Lets do it scrummy-iteratively step by step. (I still steer towards removing the IconButton abtraction in a later step ;))
|
OMG, that already looks sooooo nice. I am really impressed! |
That way we can get icons from other places in the source code as well. In the long run, we could even get rid of the IconButton wrapper, if we return the whole IconButton object via the iconBy() function and not only the icon string. Ref #48
micuintus
deleted the
jmastr/v_play_add_venue_description_implementation
branch
|
But I would at least need an Silica implementation to "fake" the |
No description provided.