hql287 / Manta

🎉 Flexible invoicing desktop app with beautiful & customizable templates.
https://www.getmanta.app
GNU Lesser General Public License v3.0
5.28k stars 478 forks source link

Sidebar overflow fixes #257

Closed ghost closed 6 years ago

ghost commented 6 years ago

Description, Motivation and Context

There were some overflow problems if the window is too small:

Fix:

Related Issue

246

How Has This Been Tested?

Changes were tested on Ubuntu Gnome 16.04 by resizing the preview window from maximized down to 100px x 100px.

Screenshots (if appropriate):

These 3 scrollbars are the maximum, should you see more scrollbars in some case it's a bug: bildschirmfoto-20180307200642-1916x1066

if the window is wide enough for the content there shouldn't be more than 2 scrollbars: bildschirmfoto-20180307200850-2549x1069

no sidebar element should get squished anymore: bildschirmfoto-20180307201034-2573x1340

Types of changes

Checklist:

ghost commented 6 years ago

During testing i experienced some interesting behavior: If you open the preview window the first time after app start it has a min-size, if you open it for the second time, it has not.

Info: the actions container isn't sticky yet, i thought we first check the overflow behavior with more sidebar content :)

hql287 commented 6 years ago

@jens-t: Working great so far 👍. Still has some (small) issues though:

screen shot 2018-03-08 at 10 50 54

Will try to make a fix for this.

ghost commented 6 years ago

I think the sidebar is a bit too wide at the moment (Yes, this is subjective).

Absolutely, it's just for showing how it looks, since it's simple to change that, i thought we discuss a good sidebar width here.

When scroll, here's what it looks like on macOS:

that's because of the min-content. i can also fix this, if you do, you should consider, that this version is the wanted behavior on win & linux. so, the extra space should only apply on macOS.

hql287 commented 6 years ago

Absolutely, it's just for showing how it looks, since it's simple to change that, i thought we discuss a good sidebar width here.

@jens-t: I think the sidebar's width should be something between 200px and 215px. Eventually, I would like to make it resizable and this shouldn't be a problem anymore.

that's because of the min-content. i can also fix this, if you do, you should consider, that this version is the wanted behavior on win & linux. so, the extra space should only apply on macOS.

Yup, I noticed the difference of how electron render the titleBar on macOS vs Linux and Windows. This does make thing a little bit tricky though 🙈

Will get back to this after releasing the new update.

ghost commented 6 years ago

Will take a closer look at this tomorrow ...

ghost commented 6 years ago

added some functionality for the sidebar macOS window controls problem. there's a new wrapper element for that, so here is the new place to define the width of the sidebar. the 280px width of new SidebarWrapper matches the former 215px of the OverflowWrapper.

You can change the height of the top margin for the macOS window controls here. can't test if the 30px fit. theres probably more space needed.

i think this fix is the best way to handle the problem, what do you think? Maybe you can provide a screenshot of how this looks on macOS?

ghost commented 6 years ago

Do we still want to make the actionbuttons sticky? If yes, i would take a look at that, too.

hql287 commented 6 years ago

If you open the preview window the first time after app start it has a min-size, if you open it for the second time, it has not.

Manta remembers the size & position of every window across sessions. Unless you meant something else.

i think this fix is the best way to handle the problem, what do you think? Maybe you can provide a screenshot of how this looks on macOS?

Testing it now!

hql287 commented 6 years ago

@jens-t: Looks like it's working. Nicely done 👍

On a second note, I'm thinking about making the title bar visible on macOS as well, so that it will look more consistent compared to Windows and Linux version.

screen shot 2018-03-14 at 12 42 37

Maybe this will be helpful for future UI customization as well. Any thought? 🧐

hql287 commented 6 years ago

Update

You can change the height of the top margin for the macOS window controls here. can't test if the 30px fit. theres probably more space needed.

30px is a little tight. I tried to increase this but it somehow still doesn't feel very natural to me. I think there are some UI changes need to be made here to the sidebar. Right now we have a few issues which haven't been addressed such as:

Will play around with Sketch a bit and get back to you on this tonight!

ghost commented 6 years ago

Manta remembers the size & position of every window across sessions. Unless you meant something else.

I meant that the first time you open the preview window you can just resize the window to a certain min-size (1024px x 800px, for example), when you open it the second time you can resize it to, for example 20px x 20px, so the second time there is no min-size for the window.

On a second note, I'm thinking about making the title bar visible on macOS as well, so that it will look more consistent compared to Windows and Linux version.

If thats the case, we wont need this fix, or?

Maybe this will be helpful for future UI customization as well. Any thought?

I think it's way less hacky, so yes i think it is a good idea to streamline the experiences, if it's an option for you.

The Accent Color component's colour picker is "floating" at the moment. The Use custom color checkbox should belong to the toggle component.

I don't really understand whats the problem or the desired change, can you explain this in more detail or other words? :see_no_evil: :laughing: So that i can make these changes ... annotated screenshots or sketchscribbles may help.

hql287 commented 6 years ago

@jens-t : Sorry for not being clear.

I don't really understand whats the problem or the desired change, can you explain this in more detail or other words? 🙈 😆

What I meant with the accent color is that the color picker has "position: absolute" so even when we fix the overflow problem, this will still be an issue. That's why I think we should refactor it. Make sense? 😅

I think it's way less hacky, so yes i think it is a good idea to streamline the experiences, if it's an option for you.

Ok, let's make the title bar visible on macOS as well.

If thats the case, we wont need this fix, or?

We still do need this fix though, except the patch for macOS. I think it should be fixed with better design/UI. Just spent some time with Sketch but didn't like what I got, so I guess we should just proceed to fix the overflow issue for now.

I would like to spend more time to think about how we should handle the buttons.

screen shot 2018-03-15 at 00 06 38 screen shot 2018-03-15 at 00 06 44 screen shot 2018-03-15 at 00 05 16
hql287 commented 6 years ago

I'll now send a new PR to make the title bar visible, then you can take it from there. Sounds good?

ghost commented 6 years ago

What I meant with the accent color is that the color picker has "position: absolute" so even when we fix the overflow problem, this will still be an issue. That's why I think we should refactor it. Make sense?

Yeah it's not ideal how the color picker popup currently works. You could also display it at the top of the color field, as a quick temporary fix. But you could also scroll down to reach it in the current state. As we're designing the edge case here, it's maybe temporary negligible. In general you woudn't resize the preview window to a size that small, i think :)

We still do need this fix though, except the patch for macOS.

Yes, that's what i meant.

I think it should be fixed with better design/UI. Just spent some time with Sketch but didn't like what I got, so I guess we should just proceed to fix the overflow issue for now.

Ok.

I would like to spend more time to think about how we should handle the buttons.

Ok, it's not that urgent to change the behavior we have now. (cause edge case and so on ...)

Speaking of a kind of redesign of the sidebar, i have some experience in ui design :) do you mind if i have a take on a sidebar redesign? You can of course dismiss it, or use it as inspiration ... or as a demo of what my take on something like this would look like.

In general, considering the design of the app, there is one thing i don't really like. It's the styling of the form elements. We could give them a more consistent design across the different platforms, by styling them with a custom design that fit's the app design. we could style checkboxes as toggle when appropriate, add animations for state changes and so on ... all with some lines of css. Maybe you like it the way it is, this would be ok for me. But if not, we should probably also talk about a new design of the form elements across the whole app, consistency should be important in that regard.

And, as designing something in a group, is a tricky thing, i wouldn't mind if you want to handle the design of the app alone. It's just a proposal ...

I'll now send a new PR to make the title bar visible, then you can take it from there. Sounds good?

Yes! :+1:

hql287 commented 6 years ago

Yeah it's not ideal how the color picker popup currently works. You could also display it at the top of the color field, as a quick temporary fix. But you could also scroll down to reach it in the current state. As we're designing the edge case here, it's maybe temporary negligible. In general you woudn't resize the preview window to a size that small, i think :)

I also just refactored the colour picker component and will send a PR soon. I think it'll solve this issue. Would you like to wait for that PR first or finish this PR instead?

Speaking of a kind of redesign of the sidebar, i have some experience in ui design :) do you mind if i have a take on a sidebar redesign? You can of course dismiss it, or use it as inspiration ... or as a demo of what my take on something like this would look like.

In general, considering the design of the app, there is one thing i don't really like. It's the styling of the form elements. We could give them a more consistent design across the different platforms, by styling them with a custom design that fit's the app design. we could style checkboxes as toggle when appropriate, add animations for state changes and so on ... all with some lines of css. Maybe you like it the way it is, this would be ok for me. But if not, we should probably also talk about a new design of the form elements across the whole app, consistency should be important in that regard.

This is great! I'm always open about this 👍 Please feel free to give it ago and we can then discuss it further.

And, as designing something in a group, is a tricky thing, i wouldn't mind if you want to handle the design of the app alone. It's just a proposal ...

Maybe we can try to do it here first, if things got too complicated then we could consider taking it to other platforms.

hql287 commented 6 years ago

I also just refactored the colour picker component and will send a PR soon. I think it'll solve this issue. Would you like to wait for that PR first or finish this PR instead?

Ok, just refactored AccentColor and merged the changes to your PR.

ghost commented 6 years ago

Ok, just refactored AccentColor and merged the changes to your PR.

Nice!

This is great! I'm always open about this +1 Please feel free to give it ago and we can then discuss it further.

Will do :)

Maybe we can try to do it here first, if things got too complicated then we could consider taking it to other platforms.

Yeah, let's try it here first. With some kind of design PR's ...

ghost commented 6 years ago

Ok, just refactored AccentColor and merged the changes to your PR.

Had a look at this. Works for me. But now we have a sidebar height that also needs a scrollbar if i maximize the preview window. I think we should try the sticky actions idea before merging, will add a commit for this ...

On my os the border of the hex input is gone when in focus:

is that a problem on macOS too?

Anyway, i think we should customize the design of the color picker when redesigning. Looks like a custom component based on react-color gives more styling options: https://casesandberg.github.io/react-color/#create

So it's ok for now.

ghost commented 6 years ago

Added the sticky behavior to the action buttons, wasn't as easy as i thought to have the section stick to the bottom of the sidebar when there is no overflow.

Take a look at it and if it's ok for you this PR can be merged i think.

bildschirmfoto-20180315215844-542x819 bildschirmfoto-20180315215927-589x792

ghost commented 6 years ago

I thought a bit about your sketch screenshots and think grouping is the way to go to free some vertical space in addition to the toggles for logosize and accentcolor. Maybe something like this:

or

hql287 commented 6 years ago

On my os the border of the hex input is gone when in focus, is that a problem on macOS too?

Unfortunately, yes 😄 I think it's has something to do with Manta rather than the component it self. Creating a new one is also a great option 👍. I like to have a color wheel there (like Sketch's colour picker) but it looks too clunky at the moment.

Added the sticky behavior to the action buttons, wasn't as easy as i thought to have the section stick to the bottom of the sidebar when there is no overflow.

I know, it's kinda tricky isn't it 😜

I thought a bit about your sketch screenshots and think grouping is the way to go to free some vertical space in addition to the toggles for logosize and accentcolor.

I think it could be helpful in the future when we have too many configs, but it shouldn't be the solution for the sticky buttons.

But anyway, I thought about it more and was wondering what if we could avoid this situation altogether. Then I came up with this:

previewerbottom

Let me know what you think.

hql287 commented 6 years ago

Here's how the Previewer looks like:

previewer

hql287 commented 6 years ago

On macOS, the bottom bar is usually called status bar. With this new status bar, we can even do things such as adding shortcuts to other actions in the future via icons:

icons

Or go dark 😎

darktheme

hql287 commented 6 years ago

On improving the colour picker or creating a new one, I have a few suggestions:

ghost commented 6 years ago

Unfortunately, yes 😄 I think it's has something to do with Manta rather than the component it self. Creating a new one is also a great option 👍. I like to have a color wheel there (like Sketch's colour picker) but it looks too clunky at the moment.

Ok, will get to that later in a seperate PR.

I think it could be helpful in the future when we have too many configs, but it shouldn't be the solution for the sticky buttons.

The grouping has nothing to do for me with the sticky buttons. Just asked this thinking about a redesign of the sidebar options.

Then I came up with this: Let me know what you think.

I havn't understand yet, what problem the bottombar is solving. And there is also no bottom bar concept in windows or linux apps, so i think it kind of reminds me of the top bar macOS thingy from the beginning of the thread. but maybe thats part of a bigger question: Should the app-design relate to the macOS design or do we want to create a design that works across the different platforms in the same way, and has it's own look that also works and looks the same across platforms?

That doesn't mean that the bottom bar is a bad idea in all cases, maybe it's part of a concept i dont understand yet.

And does that mean that the actions in the main window are also moved to the bottom? To get the behavior more consistent across the app? Just thinking ...

On improving the colour picker or creating a new one, I have a few suggestions ...

Makes sense, i noted these points for later.

hql287 commented 6 years ago

I havn't understand yet, what problem the bottombar is solving

Ahh, ok. Let's go back to the beginning and address the problems that we're dealing with. From the way I see it, we have the following issues:

I think you've already solved the first issue. 👍

The second one is more tricky as we have to make it visible and also go with the flow of the sidebar when scrolling. The solution that I proposed earlier can solve this because:

And there is also no bottom bar concept in windows or linux apps

Maybe it's known by other names that I'm not familiar with but having used both Linux and Windows in the past I know that they do exist. For example:

On Windows

On Linux

screen shot 2018-03-16 at 20 47 00

But let's make things easier by NOT calling it status bar because it just happens to appear at the bottom of the window, where the status bar would appear, that's all. (The bar that I added doesn't even display any status).

So, let's call it a bottom bar as you suggested.

but maybe thats part of a bigger question: Should the app-design relate to the macOS design or do we want to create a design that works across the different platforms in the same way, and has it's own look that also works and looks the same across platforms?

Definitely the latter – a design that works across the different platforms in the same way, and has it's own look that also works and looks the same across platforms.

Btw, putting buttons at the bottom of a window is very common on many platforms, including Windows and Linux. I assume it's the gradient that makes it looks like something belongs to macOS? 🧐

And does that mean that the actions in the main window are also moved to the bottom? To get the behavior more consistent across the app?

Haven't thought about it yet. I tried putting the buttons of the previewWindow on top but it doesn't look right. That's why I put it on the bottom.

I'll try your suggestion and let you know how it goes.

hql287 commented 6 years ago

Just noticed that @SkyzohKey is secretly watching this thread! 😄 Would love to hear your thoughts on this.

ghost commented 6 years ago

The second one is more tricky as we have to make it visible and also go with the flow of the sidebar when scrolling ... It moves the two buttons to a different place where they're always visible.

I think that's also solved in the current version of the PR with a later commit, that's why i wondered about the bottom bar. Does the current version not work for you, is it buggy on macOS? For me it is working great. Design is something that can be changed, i mean the sticky behavior itself.

I also thought the design doesn't have to be perfect for now, cause we're discussing a greater redesign ... at least it fits the current design state of the app :)

Definitely the latter – a design that works across the different platforms in the same way, and has it's own look that also works and looks the same across platforms.

Ok, good to know :+1:

Btw, putting buttons at the bottom of a window is very common on many platforms, including Windows and Linux. I assume it's the gradient that makes it looks like something belongs to macOS?

Maybe you're right :smile: And maybe also the blue/white button combo ... i thought a bit more about the button bar, and maybe we should do some tests with it :)

Some things, that are not that ideal for me:

But the extra space for new prominent elements could be a good idea for the mainWindow, don't know what to place there in the previewWindow, i think in the preview window belongs only preview stuff. Or are there new preview features coming that need that space?

The other good thing is that you can display the buttons next to each other, but the question is, is it worth the disadvantages.

I'm not sure yet, what's my final opinion on that :smile:

Haven't thought about it yet. I tried putting the buttons of the previewWindow on top but it doesn't look right. That's why I put it on the bottom.

Yeah, if it's 100% window with, i also think at the bottom is better.

hql287 commented 6 years ago

And maybe also the blue/white button combo

That's true! 😄

or that it's a long way from a setting in a sidebar to the export button

I really don't think this is an issue.

In my opinion, this may not be the perfect solution but it does solve all of our issues with one small disadvantage that you mentioned, which is smaller viewport for previewing the whole invoice. For this, I think we can we can introduce a new feature to allow the user preview the invoice in fullscreen mode. Maybe something like this:

fullscreen

Anyway, if you still want to explore other solutions, feel free to take the time to work on it. We can keep this PR open until then.

Or we can choose this as a "good enough" solution at the moment to finish this PR and move on. Then maybe get back to it at some other time in the future.

Let me know what you think.

ghost commented 6 years ago

or that it's a long way from a setting in a sidebar to the export button

I really don't think this is an issue.

Ok :)

we can introduce a new feature to allow the user preview the invoice in fullscreen mode

Thats a good idea!

Or we can choose this as a "good enough" solution at the moment to finish this PR and move on. Then maybe get back to it at some other time in the future.

Yeah, this! Let's get the fix out first and let's work on a more prefect solution later :)

Anyway, if you still want to explore other solutions, feel free to take the time to work on it.

Let's work on that in a seperate PR. Maybe this could also be part of a greater redesign PR ...

Some questions about the future PR / Bottom Bar:

i would display the previewWindow bottom-bar only with the action buttons and the fullscreen icon. And the mainWindow bottom bar with all the other icons / actions ... they probably stand for settings, user data sync, new invoice? and some kind of hide somthing? :)

And: I think the bottom-bar has the potential to help with a more intuitive user flow, if we get it right. Reaching prominent settings with one click is always a good thing.

hql287 commented 6 years ago

What are the functions of the other bottom bar icons?

This's just a mockup at the moment so I don't know what actions those icons will perform, yet. They're like placeholders and we don't even need to include them in this PR. (Unless you already have something in mind)

Should both, the mainWindow bottom-bar and the previewWindow bottom bar include all these icons/actions?

No, I don't think so. Since mainWindow and previewWindow are 2 separate windows with 2 separate responsibilities, I don't see any reason to include these icons in both windows.

We might do this if both windows have shared action(s) though.

i would display the previewWindow bottom-bar only with the action buttons and the fullscreen icon. And the mainWindow bottom bar with all the other icons / actions ... they probably stand for settings, user data sync, new invoice? and some kind of hide somthing? :)

Yeah, this is where we can get creative! 😁

And: I think the bottom-bar has the potential to help with a more intuitive user flow, if we get it right. Reaching prominent settings with one click is always a good thing.

👍

Yeah, this! Let's get the fix out first and let's work on a more prefect solution later :)

So would you like to finish this PR with this proposed solution?

ghost commented 6 years ago

This's just a mockup at the moment so I don't know what actions those icons will perform, yet. They're like placeholders and we don't even need to include them in this PR. (Unless you already have something in mind)

Ah ok, understand ... thought you had already something in mind for them.

No, I don't think so. Since mainWindow and previewWindow are 2 separate windows with 2 separate responsibilities, I don't see any reason to include these icons in both windows. We might do this if both windows have shared action(s) though.

Yeah, that's what i would have suggested too.

Yeah, this is where we can get creative!

:smile:

So would you like to finish this PR with this proposed solution?

It already is, or am i missing something?

hql287 commented 6 years ago

It already is, or am i missing something?

@jens-t: Have you implemented the bottom bar yet?

ghost commented 6 years ago

@jens-t: Have you implemented the bottom bar yet?

I think we've talked past each other :laughing: i thought we talked about adding the bottom bar in a later PR ... cause adding it, means big changes in the html of the mainWindow and previewWindow, needs more design thinking, and so on ...

Maybe because we talked about "the fix", for me,"the fix" is already there, all Problems the bottom-bar would solve in another way are aleady solved in the current state. Thats why i asked if you already took a look at the current state of the PR ...

hql287 commented 6 years ago

@jens-t: Ahh, haha ok! 😄 Sorry for not making it clear. Let's do it in another PR then. 👍

(FYI, I did take a look at the current state of this PR, it's working but I thought you said it felt hacky or tricky somewhere so I thought you're not happy with it as well. That's why I proposed the bottom bar solution. Guess the conversation was a bit long so both of us got confused.😄)

Anyway, I looked at commits again and noticed that we can make it works by simply removing the flexbox effect from the sidebar completely. So it'll be just a normal div with overflow: scroll enabled. As for the sticky buttons, just make it fixed at the bottom and we're good to go.

I just refactored the code a little bit and it works exactly the same way. Please let me know if it works on Linux as well. If there's no potential side-effect then I think we can move on :)

ghost commented 6 years ago

(FYI, I did take a look at the current state of this PR, it's working but I thought you said it felt hacky or tricky somewhere so I thought you're not happy with it as well. That's why I proposed the bottom bar solution. Guess the conversation was a bit long so both of us got confused.smile)

I only said, it was tricky to get it right. With that i meant it was hard. But it was working perfectly and was a valid & not hacky solution. But now i understand the confusion.

Anyway, I looked at commits again and noticed that we can make it works by simply removing the flexbox effect from the sidebar completely. So it'll be just a normal div with overflow: scroll enabled. As for the sticky buttons, just make it fixed at the bottom and we're good to go.

I tried your solution too, but that doesn't work for linux & windows, because the scrollbar is hidden behind the actions section: bildschirmfoto-20180318125347-516x614 The scrollbars on macOS are working differently, so you couldn't see this bug. The strange thing is, that the scrollbars on linux in "not electron" apps are also working differently. Chromium uses it's own custom scrollbars on linux, not the GTK ones, thats why they are different in electron apps too. Maybe that wouldn't be so bad, if they wouldn't look that oldschool :laughing:

So, i think we should revert the last 3 commits and then we're good to go :)

ghost commented 6 years ago

FYI: this problem also occurs in the mainWindow, saw this some time ago and thought, we can fix that later: bildschirmfoto-20180318131144-626x287

hql287 commented 6 years ago

@jens-t: It's merged! Sorry for the delay, something came up and required my immediate attention in the last couple of days.

ghost commented 6 years ago

@jens-t: It's merged! Sorry for the delay, something came up and required my immediate attention in the last couple of days.

No problem! I also got ill, and am still recovering. Will get back on the Template refactoring in a couple of days ✌️

hql287 commented 6 years ago

@jens-t: Speedy recovery bro! 👍

ghost commented 6 years ago

Thanks, i'm finally healthy again, but have some other projects at the moment that need my focus. I get back to you and manta in the near future ... just fyi :)

hql287 commented 6 years ago

@jens-t: Ahh, it's good to have you back, bro 👍 No pressure, any help is greatly appreciated as always! :)