-
Notifications
You must be signed in to change notification settings - Fork 35
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
Replace NavigationRail with YaruNavigationRail #201
Conversation
This is normal :D |
Woops ok sorry for the noise then :) Looks good so far |
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 rail.mp4Since 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? |
5665a00
to
eff2883
Compare
Capture.video.du.04-10-2022.12.37.07.webm@Feichtmeier I implemented the extended mode :) |
but for the normal size I think I missed some changes for the labels? Anything else I need to change? |
Are you using |
crazyanimation.webm |
@Feichtmeier Is it better like this? Before: After: |
It is also my opinion 👍
Well, I wanted to propose you the same thing and do it in another PR after this one :D |
Yeah let's start by replicating the same look for now! So +1 for the font weight :)
Sounds nice 👍 Sorry for all the IconData and String properties everywhere 😮💨 |
No problem, all best practices can't be known from the beginning ;D |
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. |
No problem, I added a little padding top to realign the first item :)
I don't know if I had correctly understood :D |
Uh nice this is now perfect 🙂 I meant when you change from not extended to extended the size change has a transition:) |
26f842b
to
d4594ea
Compare
I am on the page item issue so you don't need to do it :) |
A transition like this: ? Capture.video.du.06-10-2022.14.57.09.webmSorry for the flickering 🙈 |
Yeah exactly! 🥳 |
And like this (width and height transition): Peek.06-10-2022.15-10.mp4Notice that I increased the compact labelled mode, and I used max-lines 1. |
from medium to small it looks good |
And like this (I changed the alignment): Peek.06-10-2022.15-24.mp4 |
For some reason it looks fine in one direction but not in the other Oo |
This time I think I got it :D Peek.06-10-2022.15-59.mp4 |
It looks really good now! From my point of view you are done :) (minus the conflicts... sorry 🙈 ) |
Okay, I still need to do a global clean up and some little nitpick, and then merging :)
Nooo, merge conflicts again 😆 |
I want to add tooltips and semantics to both navigation layouts, but I'll do it in another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this long work! looks good from my point of view!
- flutter analyze - flutter build linux - flutter format - flutter pub publish --dry-run - flutter test
Wip PR, for testing and have feedback about it.
All the code added and around it need some cleanup.
CC @Feichtmeier