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

Move percentdone bar to bottom of task #87

Merged
merged 8 commits into from
Jun 29, 2017
Merged

Move percentdone bar to bottom of task #87

merged 8 commits into from
Jun 29, 2017

Conversation

andreasjacobsen93
Copy link
Member

I think this makes much more sense visually.
I always mistake the percentdone bar for being to the task above it.

I think this makes much more sense visually.
I always mistake the percentdone bar for being to the task above it.
@raimund-schluessler
Copy link
Member

This is very much a matter of taste. When the progress bar is at the bottom, you could easily mistake the bar for belonging to the task below it.

What do you think @jancborchardt ?

@andreasjacobsen93
Copy link
Member Author

I read the text first, then see the progress bar.
I think this is less confusing.

@jancborchardt
Copy link
Member

I agree below is better. But also to avoid confusion, I would separate it visually from the dividing line, and associate it more with the title. A bit like so:

screenshot from 2017-06-20 17-26-28

@andreasjacobsen93
Copy link
Member Author

I think we should keep the full width, but I provided a little space in the bottom to separate it from the one below.

@andreasjacobsen93
Copy link
Member Author

I think it looks better if we also lose the 2px round corners, what do you guys think?
So we remove this border-radius: 2px 2px 2px 2px;

@jancborchardt
Copy link
Member

The round corners should stay because we do that for the new quota bar in Files too. And it should not be full width because that makes it less apparent it's a progress bar. :)

Check out the server master with the quota bar including grey for full width: nextcloud/server#5305 (comment)

@andreasjacobsen93
Copy link
Member Author

screen shot 2017-06-26 at 18 35 40

This could be a temporary solution, to improve the usability a little bit, and I will look into styling it more like the new quota later?

@jancborchardt
Copy link
Member

It would be cool to do it fully directly. :) The screenshot looks good but a bit off, just halfway done. Let me know if you need any help! :)

@andreasjacobsen93
Copy link
Member Author

Alright, update. :-)
capture

@andreasjacobsen93
Copy link
Member Author

A little more clean now! :-)

capture

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 27, 2017

I like how this looks now. 😃

I haven't tested it yet, but from the screenshots it looks as if the title of the task jumps up a bit if there is the percentbar present. Do we really want this? I would prefer to have the title static at the same position.

Also, before merging we need to squash all the commits, but I can do that once this is finished.

@jancborchardt
Copy link
Member

Really great work @andreasjacobsen93! :)

@raimund-schluessler yes, there is a slight positional change. It's necessary to make both states look nice, and I think it's absolutely ok.
It's the same with the Contacts app for example, where the contact name in the list is at a different height depending if secondary info like the email is present or not. :)

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 29, 2017

Alright, then I will merge this as it is.

@andreasjacobsen93 Thanks a lot for the contribution. More is welcome 😉

@raimund-schluessler raimund-schluessler merged commit a4cf0f7 into nextcloud:master Jun 29, 2017
@andreasjacobsen93 andreasjacobsen93 deleted the patch-1 branch June 29, 2017 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants