Skip to content

Conversation

@aarushi15002
Copy link

I have tried and rectified the issue : Three dots menu in files in greyed out

@mention-bot
Copy link

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

@ChristophWurst
Copy link
Member

Hey @aarushi15002,

thanks a lot for your contribution. Is there a ticket or bug report this pull request fixes? Otherwise it would be great if you could describe what problem this changes fixes. Also screenshots of before/after can be useful with design changes.
Are you the author of the code? GitHub shows that two different users authored and created the PR.

@aarushi15002
Copy link
Author

This pull request fixes a bug report. The list in the three dots menu as well as the dots were barely visible earlier but after the changes it is better.
before:
original
after changes:
changed

Also I am the author of this pull request . I have two github accounts so by mistake I made the PR from the other mail id . Sorry for the inconvenience caused.

@MorrisJobke
Copy link
Member

@ChristophWurst @aarushi15002 I guess this is the PR that should fix #3522

Copy link

@innocent-rajan innocent-rajan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to me the changes are perfectly fine.
And by seeing the error the are getting think its not the issue with their code but with the server as far as i can understand.
Please let me also know what the exact error is.

@ChristophWurst
Copy link
Member

thanks @MorrisJobke.

To me it looks best if the opacity is removed completely. cc @nextcloud/designers

@innocent-rajan
Copy link

screenshot from 2017-03-07 15-07-51

This doesn't look that good rather looks too dark. Making the opacity as 0.5 will be better.

@ChristophWurst
Copy link
Member

Okay, I wasn't clear about what I've changed. The opacity on the icons is almost fine. However, the opacity on the menu items is way to low. The text is very hard to read since the contrast is low. We might need a specific rule for that.

@aarushi15002
Copy link
Author

54ee1dcf-a1d2-4409-ab68-ef1b56d64522
Size, Modifier are defined to of the color they appeared before, which means that no opacity is associated with is. Where as if we do that then we need to change the whole website color and the contrast which makes no sense according to me.
So a better option is to make the opactity to .5 from .3 as .3 looks too dim.
This looks better i guess.
d1a1e237-7961-4059-b770-755920e2bf0d

@jancborchardt
Copy link
Member

.5 opacity seems good, great point. Please make sure to also test it on mobile as there needs to be sufficient contrast to the files which have been shared – for them the share icon will be shown a bit darker, should be full opacity.

Can you look at the linked issue #3522 to make sure that is also fixed?

Thanks! :)

@rullzer rullzer added 3. to review Waiting for reviews design Design, UI, UX, etc. enhancement labels Mar 10, 2017
@rullzer rullzer added this to the Nextcloud 12.0 milestone Mar 10, 2017
@rullzer
Copy link
Member

rullzer commented Mar 30, 2017

@jancborchardt is this still needed now that #4082 is in?

@jancborchardt
Copy link
Member

@aarushi15002 @innocent-rajan can you comment please? Make sure that this pull request is rebased on latest master changes. :)

@aarushi15002
Copy link
Author

Yeah I need to check once and then will let you know. And the code above resolves #3522.

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 4, 2017
@MorrisJobke
Copy link
Member

I just checked on this and it doesn't result in any change, because there is another rule that overwrites this one. I will close this and may have a look at this opacity mess in files.scss.

Thanks for your help @aarushi15002 anyways :) Maybe check out our other issues - the starters are quite good ones: https://github.com/nextcloud/server/issues?q=is%3Aissue+is%3Aopen+label%3A%22starter+issue%22

@MorrisJobke MorrisJobke mentioned this pull request Apr 19, 2017
@MorrisJobke
Copy link
Member

@aarushi15002 and here it is: #4386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress design Design, UI, UX, etc. enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants