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

add avatar to sharing dropdown #373

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

georgehrke
Copy link
Member

fixes #207

@mention-bot
Copy link

@georgehrke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @raghunayyar, @tcitworld and @jancborchardt to be potential reviewers.

@georgehrke georgehrke added the 2. developing Work in progress label Mar 20, 2017
@georgehrke
Copy link
Member Author

calendar - nextcloud chromium today at 11 01 01 pm

@georgehrke
Copy link
Member Author

The avatars should be added to the sharing list as well, but I'm generally not too happy about the current design of the "shared-with" list.

calendar - nextcloud chromium today at 11 02 38 pm

cc @jancborchardt @eppfel @nextcloud/designers

@georgehrke
Copy link
Member Author

I would prefer some design like this: (just like the files app)
files - nextcloud chromium today at 11 08 24 pm

but we have too little space in the navigation bar :/

@georgehrke
Copy link
Member Author

cc @nickvergessen who reported the original avatar issue

Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke georgehrke force-pushed the feature/207/add_avatar_list_to_sharing_dialog branch from 6cbcb23 to a54c491 Compare March 20, 2017 22:14
@georgehrke
Copy link
Member Author

And there is no real visual indication whether the expanded area belongs to test 2 or test 3
calendar - nextcloud chromium today at 11 15 00 pm

@tcitworld
Copy link
Member

And there is no real visual indication whether the expanded area belongs to test 2 or test 3

Maybe the whole area could have the calendar color instead of just the dot ?

@georgehrke
Copy link
Member Author

calendar - nextcloud chromium today at 12 02 31 am

That's with alpha .2

Hm. It could work, but there is no other place in Nextcloud doing it this way.
I'd prefer to use something we already use in other places of Nextcloud.

@tcitworld
Copy link
Member

Hm. It could work, but there is no other place in Nextcloud doing it this way.

Yeah well I can't think of any other part of Nextcloud where the sidebar has to be reduced this small to show the calendar aside, so whatever we choose, I guess it wouldn't be just like the files app. :(

@georgehrke georgehrke added this to the 1.6.0-current milestone Mar 21, 2017
@georgehrke georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 22, 2017
@georgehrke
Copy link
Member Author

I created an issue about the "shared-with" list: #377

@tcitworld
Copy link
Member

tcitworld commented Mar 22, 2017

Works correctly. Just a small ui issue on my side :

selection_030

@georgehrke
Copy link
Member Author

@tcitworld What browser / os is that? :)

@tcitworld
Copy link
Member

Firefox 52 on gnu/linux. Chrome/ium doesn't have this issue.

@georgehrke
Copy link
Member Author

georgehrke commented Mar 22, 2017

calendar - nextcloud firefox today at 5 08 32 pm

Looks fine with Firefox on macOS

I hate scrollbars in browsers ...

@tcitworld
Copy link
Member

tcitworld commented Mar 22, 2017

Seems to be a resolution issue. If I use the adaptive view of Firefox to change the resolution (including the one I actually use, 1920x1080), the issue is gone.

@georgehrke georgehrke merged commit 35f5629 into master Mar 22, 2017
@georgehrke georgehrke deleted the feature/207/add_avatar_list_to_sharing_dialog branch March 22, 2017 16:45
@jancborchardt
Copy link
Member

By the way, see how the share dropdown is nice and white in the files sidebar – can you take the same styles from there? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add avatar to sharing list
4 participants