Replace NavigationRail with YaruNavigationRail

[Jupi007]

Wip PR, for testing and have feedback about it.
All the code added and around it need some cleanup.

CC @Feichtmeier

[Feichtmeier]

Thanks Paul for this work!

The compact layout looks very good with the navi rail you made

However something happend with the normal layout aswell:

(left yaruwidgets demo from your branch, right local settings app with the current master)

I think the text alignment and the font size is changed :)

[Jupi007]

I think the text alignment and the font size is changed :)

This is normal :D
I modified the YaruPageItemTitle text style but this is just a temporary change (see TODO comment).

[Feichtmeier]

Woops ok sorry for the noise then :) Looks good so far

[Feichtmeier]

I wanted to see if we can change the extended property in the wide layout's navigation rail with a size from a mediaquery if the width is higher than X
Works and the transition looks cool
Extended only works if NavigarionRailLabelType.none is set - I think that's okay but I think elio wanted that the labels are always shown for accessibility reasons

rail.mp4

Since you are creating our own rail, could you add something similar to the "extended" flag? Like if extended is true make it a row, otherwise a column?

[Jupi007]

Capture.video.du.04-10-2022.12.37.07.webm

@Feichtmeier I implemented the extended mode :)

[Feichtmeier]

Works for wide

and for slim

but for the normal size I think I missed some changes for the labels?

Anything else I need to change?
Here is the branch:
https://github.com/ubuntu-flutter-community/software/pull/355

[Jupi007]

Are you using YaruPageItemTitle in the label builders?
It is required to have the correct styling :)

[Feichtmeier]

Neat!

Only the text size is smaller now, could you fix this in the default text styling for the yarutitles instead?
The smaller font also makes the whole tile be less high. Previously it was on the same level as the seach bar

[Feichtmeier]

crazyanimation.webm

[Jupi007]

@Feichtmeier Is it better like this?

Before:

After:

[Feichtmeier]

Very close :) I think there needs to be more padding at the bottom of the indicator

ANother thing is that the icon is only build from the iconData property of YaruPageItem
Which is ofc again based by yet another stupid idea of mine to not use WIdgets as properties of widgets :(
Could you change YaruPageItem to just have an icon and a selected Icon property (with both being full WIdgets)?
In software I use different "itemWidget" (...sorry) when stuff is going on like refreshing or updates are there etc

Edit: or maybe we need to do this in two different PRs

[Jupi007]

Very close :) I think there needs to be more padding at the bottom of the indicator

It is also my opinion 👍
I also noticed that the material rail is using a font weight 500 (currently 300), do you want it in our own one?

ANother thing is that the icon is only build from the iconData property of YaruPageItem
Which is ofc again based by yet another stupid idea of mine to not use WIdgets as properties of widgets :(
Could you change YaruPageItem to just have an icon and a selected Icon property (with both being full WIdgets)?
In software I use different "itemWidget" (...sorry) when stuff is going on like refreshing or updates are there etc

Edit: or maybe we need to do this in two different PRs

Well, I wanted to propose you the same thing and do it in another PR after this one :D
I plan to create some animated icons (to use when we select an item in software sidebar), and so we need to pass a widget and not an IconData.

[Feichtmeier]

Yeah let's start by replicating the same look for now! So +1 for the font weight :)

Well, I wanted to propose you the same thing and do it in another PR after this one :D
I plan to create some animated icons (to use when we select an item in software sidebar), and so we need to pass a widget and not an IconData.

Sounds nice 👍

Sorry for all the IconData and String properties everywhere 😮‍💨

[Jupi007]

Sorry for all the IconData and String properties everywhere face_exhaling

No problem, all best practices can't be known from the beginning ;D

[Jupi007]

Done:

[Feichtmeier]

Font looks good now! The sizing is still a bit different, even if this is ultra nit-picking by me. Currently the first page item selector is exactly centered with the yaru sized appbar, which is not yet pefectly the same here (sorry for being so horrible here)

Is it possible to also add the transition between very wide and medium layouts?

How about detaching this change from compact layout for now so we can soon get this in.
Then in another PR we could replace the material navigation rail inside compact layout with this?
Or would it be too much effort to detangle this now?

Edit: here is a screenshot from the current material rail

[Jupi007]

Font looks good now! The sizing is still a bit different, even if this is ultra nit-picking by me. Currently the first page item selector is exactly centered with the yaru sized appbar, which is not yet pefectly the same here (sorry for being so horrible here)

No problem, I added a little padding top to realign the first item :)

Is it possible to also add the transition between very wide and medium layouts?

How about detaching this change from compact layout for now so we can soon get this in. Then in another PR we could replace the material navigation rail inside compact layout with this? Or would it be too much effort to detangle this now?

I don't know if I had correctly understood :D
You mean wrap the transition between extended/non-extended mode in YaruCompactLayout?

[Feichtmeier]

Uh nice this is now perfect 🙂

I meant when you change from not extended to extended the size change has a transition:)

[Feichtmeier]

I am on the page item issue so you don't need to do it :)

[Jupi007]

A transition like this: ?

Capture.video.du.06-10-2022.14.57.09.webm

Sorry for the flickering 🙈

[Feichtmeier]

Yeah exactly! 🥳

[Jupi007]

And like this (width and height transition):

Peek.06-10-2022.15-10.mp4

Notice that I increased the compact labelled mode, and I used max-lines 1.
Imo, multiple lines looks weird, and we should have only one line, and a tooltip.
Wdyt?

[Feichtmeier]

from medium to small it looks good
from medium to large or vice versa I would only use the horizontal transition

[Jupi007]

And like this (I changed the alignment):

Peek.06-10-2022.15-24.mp4

[Feichtmeier]

For some reason it looks fine in one direction but not in the other Oo

[Jupi007]

This time I think I got it :D

Peek.06-10-2022.15-59.mp4

[Feichtmeier]

It looks really good now! From my point of view you are done :) (minus the conflicts... sorry 🙈 )

[Jupi007]

It looks really good now! From my point of view you are done :)

Okay, I still need to do a global clean up and some little nitpick, and then merging :)

(minus the conflicts... sorry see_no_evil )

Nooo, merge conflicts again 😆

[Jupi007]

I want to add tooltips and semantics to both navigation layouts, but I'll do it in another PR.

[Feichtmeier]

Thanks for this long work! looks good from my point of view!