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

Why? What? Who? How? #1

Closed
LuchoTurtle opened this issue Feb 14, 2023 · 5 comments
Closed

Why? What? Who? How? #1

LuchoTurtle opened this issue Feb 14, 2023 · 5 comments
Labels
discuss Share your constructive thoughts on how to make progress with this issue documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues learn Learn this!

Comments

@LuchoTurtle
Copy link
Member

Why?

As we're building our mvp, and doing so in Flutter, we want the onboarding experience to be as seamless and as easy as possible.
This concept of Progressive UI/UX will be exemplified in this repo, where navigation will only be shown after a certain action.

The navigation will open up to a full screen, where user can be shown which item to go for and redirecting to the page accordingly.

For more information about why this repo exists, please check dwyl/app#305.

@LuchoTurtle LuchoTurtle added documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality learn Learn this! flutter Flutter related issues discuss Share your constructive thoughts on how to make progress with this issue labels Feb 14, 2023
LuchoTurtle added a commit that referenced this issue Feb 14, 2023
LuchoTurtle added a commit that referenced this issue Feb 14, 2023
@LuchoTurtle LuchoTurtle moved this to 🏗 In progress in dwyl app kanban Feb 15, 2023
@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Feb 15, 2023

The issue I'm having with this is because of what's asked in the UI at dwyl/app#305.
The navbar is explicitly static and persistent when transitioning between the menu and the page.

I'm keeping the code samples simple on purpose as I want to focus on the best course-of-action, not necessarily implementation details.

Way 1

My obvious first step was trying to use the drawer property in the Scaffold. It's easy and it works, we get a simple drawer with an animation.

image

This can be resized to fit the whole screen. The issue is that the AppBar is overridden. This is not what's asked in dwyl/app#305 and, counter-intuitively, is not what's specified in Material Design's specification.

I tried getting drawer to just be part of the body of the page, below the AppBar. Even though this is doable, it leads to problems as it requires nested scaffolds.

See:

I could alternatively just add a Padding to the drawer but the result is not the same (and also feels like a "hacky" way of resolving this issue).

Way 2

Having this problem, I tried another avenue of not using Drawer with Scaffold.
This required stepping into Animation to get the same effect as a Drawer.

Not going to go into implementations details here, but this would basically require
to have a Stack on every page, to show the menu.

      body: Stack(
        children: [
          AnimatedBuilder(
            animation: _menuSlideController,
            builder: (context, child) {
              return FractionalTranslation(
                translation: Offset(1.0 - _menuSlideController.value, 0.0),
                child: _isMenuClosed() ? const SizedBox() : const SlidingMenu(),
              );
            },
          ),
        ],
      ),

This yields the following result.

330260234_5702550336535123_8920217462815251108_n.mp4

It may seem the correct solution but I notice a clear performance difference when compared with the Drawer. Drawer's animation is much more fluid whilst this one "lags". You can't see in the video due to the frame-per-second I can record on my phone but this is what happens.

Way 3

To try and fix the issue of not having to use a Stack on every page, I tried to persist the AppBar between different pages by passing each page as a child to the Main widget, thus rendering each one inside the Main one.

This only works with named routes.

      builder: (context, child) => HomePage(child: child),
      initialRoute: '/',
      routes: {
        '/': (context) => HomePage(),
        '/second': (context) => SecondRoute(),
      },

However, two problems still arise with this option:

As per what was discussed in standup, @nelsonic stated that using named routes should not be considered due to performance issues that may arise in the future. With option 3 (and by consequence, option 2), all it remains is option 1 (from what was attempted).

I don't know what other avenue I should try or if I should attempt to try another way, even though it feels "hacky" at this point.

If you want to give an opinion on this @nelsonic , go for it.

@nelsonic
Copy link
Member

I'm actually not concerned about having the "App Bar" visible when the Nav Menu is activated.

image

In fact What HeyFlutter.com demonstrates in this Navigation Drawer tutorial is almost exactly what we want:
https://youtu.be/17FLO6uHhHU
image

If there is a widely viewed tutorial for doing this I don't consider it to be "difficult" to implement.
The only "hard" part is the full-width.

My reading of: https://stackoverflow.com/questions/54227094/flutter-drawer-width-modification
is that setting the the following code would make it full-width:

drawer: Container(
    width: MediaQuery.of(context).size.width * 1.0,
    child: Drawer()),

The Navigation Drawer seems like the best Widget for this creating our Nav Menu.
Not a fan of the name "Drawer" considering the widget overlays the rest of the App screen.
Strong preference for having it full-screen so that demands the full focus of the person.

But ... quite happy to UX-test this real people to find out
if they "miss" having the 15% of noise under the menu.
as in the case of the GMail App:

image

Nobody is going to call-out Google for their poor UX. But more people should! 🙊
After all they "invented" Material UI that is ubiquitous.
But they also invented Google+ ("Plus") ... 😕
and hundreds of other failed products: https://killedbygoogle.com

image

Google gets things "wrong" a lot more than they get them "right".
They just have a Ad Biz Mega Cash Cow that allows them to keep making mistakes in most of their other products.
Don't be "afraid" of questioning UI/UX just because Google or "XYZ Unicorn is doing it".

For future reference:
if something in a Figma sketch is not immediately easy to implement in Flutter code,
then please propose a change to the Figma
don't waste hours trying to hack the Flutter Widget to look like the wireframe.

There will be exceptions to this in the future where we want the UI to be "innovative" (i.e. non-standard)
but for now, this kind of "boring" UI widget should be as close to "stock" as possible
so that it performs consistently on as many devices as possible.

Also, for future reference: this isn't a matter of "opinion".
I'm trying to think about all of this pragmatically from first principles
not blindly accept the dogma handed down by silicon valley.

Not suggesting that's what you're doing, don't worry.
It's good that you're working your way through figuring all of this out. 👍
Keep up the good work. Ask way more questions. 🙏

@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Feb 15, 2023

You are right, implementing a simple drawer the way you want to (now that it was clarified) is easy to implement. Making it full-width is simple, as well. It's what I did first - see 7d7d118

My reluctance to even use Drawer initially stemmed from exactly what you've just written, about it being a UX anti-pattern that I've seen you mentioning several times. But I noticed that I could use it just semantically and make it viable.

What wasn't clear to me and was actually the main point of my deviating greatly from what you originally intended was the AppBar. The original issue in dwyl/app#305 doesn't really specify this, but one can easily incorrectly infer from the picture that is supposed to be shown and persisted while transitioning (the AppBar is the same in both screens).

image

Again, I should have asked to clarify but, to me, it seemed "evident" enough that it you wanted it to be that way (turned out, after all this time, it wasn't).

Personally, to me, after you've introduced this picture:

image

It made much more sense and it's much clearer for me. The AppBar persistence to me was "the root of all evil", as it incurs ramifications, particularly pertaining to Navigation, that was only made possible through named routes, which was previously mentioned as a "no-go" and that I spent most of my time trying to go around it.

Regardless, in the time between me asking for "the way to go" in #1 (comment) and you answering, I've gone through a way of something akin to a drawer but tries to persist the AppBar (similar to "Way 3"). This obviously deviates from what you want. But I won't let it go to waste. I'll showcase in the demo this way of doing things and the pros and cons.

So, as for now, I'm doing the finishing touches to make the AppBar as close to what the original wireframe intended (with the avatar included).
After that's done, I'll produce the simpler version of the drawer you wanted all along.

I don't find this time wasted because now we'll have documented two different ways, even if we go for one, the other touches on topics that are important, which concern withNavigation, that we will certainly have to deal with along the way.

Lesson learned on communication and probably overthinking on my part.
Live and learn, I guess 👍

@nelsonic
Copy link
Member

Apologies for delayed reply. I had 80% of this written before lunch
and then got called away and only got back to my desk 15:00 ... ⏳ 🤦‍♂️
Glad you figured it out and made progress.
Please log your progress more often in issue comments
and any time you get stuck ping me a direct link to the comment on Signal so I can "unblock". 🙏

LuchoTurtle added a commit that referenced this issue Feb 16, 2023
LuchoTurtle added a commit that referenced this issue Feb 16, 2023
LuchoTurtle added a commit that referenced this issue Feb 16, 2023
@LuchoTurtle LuchoTurtle moved this from 🏗 In progress to ⏳Awaiting Review in dwyl app kanban Feb 16, 2023
LuchoTurtle added a commit that referenced this issue Feb 16, 2023
@nelsonic
Copy link
Member

I think we can consider this done. ✅

@github-project-automation github-project-automation bot moved this from ⏳Awaiting Review to ✅ Done in dwyl app kanban Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Share your constructive thoughts on how to make progress with this issue documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues learn Learn this!
Projects
Status: Done
Development

No branches or pull requests

2 participants