[mobile ui] navigation rework

[Dirk Hohndel]

Sebastian and Marco (welcome to the team, btw):

Awesome. The jump forward of the mobile UI is stunning. It's fun to read
through the commits and see the two of you work together.

Thanks for the time and effort spent on Subsurface-mobile this weekend!

The commits have a tiny bit of back-and-forth in them (some times things
get added and then later deleted... maybe the various syncs with
mobilecomponents could be merged into one...) but instead of spending time
to artificially rewrite history into something more pleasing I figured
"what the heck, this is awesome" and left them untouched from what you had
in your branch. And since you didn't touch compiled code (except for
adding the //imports) I didn't see any concern in doing this, anyway -
this is truly just moving the mobile UI forward.

On Mon, Nov 30, 2015 at 02:35:49AM +0000, Sebastian Kügler wrote:
> 
> Pull again. We did a whole bunch of fixes in the components and the 
> applications. The regressions that you saw should be fixed, at least the
> one we saw as well.

Yes, most of the issues I saw are addressed in master now. New Android
APKs have been uploaded for testing. Those of you with access to an
Android device: seriously, check this out.

> > When running this on Linux I get quite a few warnings and errors and can
> > easily get things to crash (Add Dive - that's a known issue and not
> > something I'm overly worried about right now).
> 
> I haven't looked into crashers, but it doesn't seem more crashy than before. 

No, I wasn't trying to imply that you made it any worse. More stating that
this is still an unsolved issue. Add Dive needs some more love :-)
I started on this but then got distracted with other stuff... I need to
resurrect that.

> Definitely something we need to look into, but I think we first need to decide 
> which options are staying in for a first beta, and which one's we'll remove (or 
> only show conditionally).

I'd have no problem with hiding the "Add dive" for a first beta. It's
definitely something that I know people will want, but it's not critical
to have in a first beta.

Right now I feel like I'm a little too close for good judgment. I'll need
to play with the app some more, play with the GPS features, install it on
new devices, go through the setup, all that good stuff. Then I'll know
more how I want things ordered in the menu.

> Now that we have the navigation bits in place, I'd like to go over the list of 
> menu items one by one and decide what to do with them, and then sorting the 
> remaining ones.

Agreed. See above.

> > qrc:////imports/org/kde/plasma/mobilecomponents/PageRow.qml:148:9: QML
> > ScriptAction:
> > qrc:////imports/org/kde/plasma/mobilecomponents/PageRow.qml:150: TypeError:
> > Cannot read property 'page' of undefined <Unknown File>: QML StateGroup:
> > Can't apply a state change as part of a state definition. <Unknown File>:
> > QML StateGroup: Can't apply a state change as part of a state definition.
> > <Unknown File>: QML StateGroup: Can't apply a state change as part of a
> > state definition.
> 
> We still have this and I think two other warnings, they come from the 
> mobilecomponents, and we'll fix it there.

OK

> The assets that are complained about should now also be found, the current 
> mobilecomponents branch is starting to look very solid.

Yes, the assets are there. I still get a few warnings / errors:

QQmlExpression: Expression qrc:////imports/org/kde/plasma/mobilecomponents/private/ActionButton.qml:102:17 depends on non-NOTIFYable properties:
    ActionButton_QMLTYPE_112::transform
qrc:////imports/org/kde/plasma/mobilecomponents/PageRow.qml:148:9: QML ScriptAction: qrc:////imports/org/kde/plasma/mobilecomponents/PageRow.qml:150: TypeError: Cannot read property 'page' of undefined
qrc:////imports/org/kde/plasma/mobilecomponents/private/PageStack.js:58: Error: Cannot navigate so that the resulting page stack has two consecutive entries of the same page instance.
qrc:////imports/org/kde/plasma/mobilecomponents/private/PageStack.js:58: Error: Cannot navigate so that the resulting page stack has two consecutive entries of the same page instance.
qrc:////imports/org/kde/plasma/mobilecomponents/private/PageStack.js:58: Error: Cannot navigate so that the resulting page stack has two consecutive entries of the same page instance.
qrc:////imports/org/kde/plasma/mobilecomponents/private/PageStack.js:58: Error: Cannot navigate so that the resulting page stack has two consecutive entries of the same page instance.

These last few were when I looked at a dive, then at the next dive, etc.

<Unknown File>: QML StateGroup: Can't apply a state change as part of a state definition.
qrc:////imports/org/kde/plasma/mobilecomponents/private/ActionButton.qml:52: TypeError: Cannot call method 'close' of null
<Unknown File>: QML StateGroup: Can't apply a state change as part of a state definition.
qrc:////imports/org/kde/plasma/mobilecomponents/private/ActionButton.qml:52: TypeError: Cannot call method 'close' of null

Not sure what triggered those.

> > That last one happens when I click on the black square at the bottom
> > (which I assume is supposed to be some cute button with a circle around
> > it, it just doesn't get rendered correctly.
> 
> The black square is fixed, missing icon asset.

Yep, fixed. Nice new drawer icon instead.

On Mon, Nov 30, 2015 at 02:44:00AM +0000, Sebastian Kügler wrote:
> 
> Yes, exactly. The idea is that you can use this drawer (either through the 
> button you've found, or by dragging in from the left edge and get at the 
> global app menu. It's beautiful and inviting and offers quick access to all 
> kinds of "pages" inside the app, and some quick access stuff (see the checkbox 
> for the gps service in the latest state).

I completely agree with "beautiful and inviting"
This is a million time better than the silly menu that we had before.

The GPS service item at the bottom is smart. I think this is now
duplicated, though, it seems to still be in the GPS submenu as well.
Oh, but that one no longer works, so we just need to drop that.

> You can navigate back by swiping back to the dive list now. We can still make 
> the dive details also scroll between the dives, although in my testing, this 
> swiping back and selecting the next or previous dives worked pretty smoothly. 
> I'm unsure about this right now.

I so far have only tested this on the desktop, building the APK right now
to test on my Android phone.

> In the last round of patches, I'm removing the top-right menu, since it's now 
> quite out of place, and everything has a more logical place in the general flow 
> of the ui. This also means that the top bar could get a bit smaller (I think 
> it still has some mysterious raison-d'etre), but it now doesn't have to be the 
> suitable high for an easy to hit fat-finger-target. Also, Android has a back 
> button by default everywhere, so we don't need to provide our own.

As I said above, completely agree with that move.

> Take it for a ride, I'm sure there are a bunch of glitches that we haven't 
> spotted, feedback is most welcome.

OK, the APK finished building...

Beautiful indeed. Here are somewhat random, unsorted comments:

- startup still is fairly slow; this could be because I have 400+ dives in
  the cloud storage... but either way, it would be great if we could have
  a busy indicator and not just a black screen
- preferences screen: for me (Nexus 6p) the combination of font size and
  screen size means that my email address (and the black balls for the
  hidden password) are longer than the entry area. That looks a bit silly.
  I think just moving the "(meters)" and "(minutes)" in a second row for
  those labels would fix this, but I haven't played with that.
- dive list: the visual separation between the different dives is a bit
  subtle, I think. Yes, there is the very faint line but to me after a bit
  of scrolling it took my eyes a moment to settle and parse. Maybe we can
  have alternating background colors (light blue and the current light
  gray for example)?
- opening the drawer: at least on the Nexus 6p with Android 6.0 it took
  quite a bit of trial and error until I was able to open the drawer just
  by swiping from the side. Could be a user issue. The button makes it
  easy, of course - that's a really smart UI idea. I hadn't seen that
  before so I'm not sure I would have immediately gotten this, though.
  At some point in the future it would be neat to have the "first run"
  mode where it explains the UI elements to the user.
- dive view: the profile clearly isn't perfect yet. It's not even good,
  yet. Maybe the biggest visual issue
- dive view: the Notes: field seems quite small on my device - I get 1.5
  lines of text there. That seems odd. Since we already scroll verticall
  for this view, why not have the full text visible?
- dive view navigation: I can see the logic behind a swipe from the left
  bringing us back to the dive list, but the back button does that as
  well. I still would much rather see swipes to the left and right bring
  up the next and previous dive, respectively

OK, that was quite a bit of feedback. But let's please keep this in
perspective. What we have right now is a HUGE leap forward. This is very
close to being ready to be called a beta release.

Thank you so much, again, for the effort you spent on this. I can't wait
to see a couple of the more obvious (and hopefully easy to fix) issues
addressed, get a lot more testing with more devices, resolutions, etc. But
I think we can open this up to more users pretty darn quickly. Which means
that I need to set up a Google Play account so this can be installed from
the play store (eventually - maybe we'll do the first beta just as
download from the website).

I can't wait to see Tomaz' progress on getting this to run on iOS as well
:-)

Things are looking very exciting right now.

/D