-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(share): icons #16418
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
base: master
Are you sure you want to change the base?
fix(share): icons #16418
Conversation
|
@alperozturk96 What is the webUI displaying? Is it also hiding the element or are we missing something on the client-side? |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
/backport to stable-3.36 |
55e5326 to
0761b36
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
If you want a declarative way, current you'd need to set up the Grey color we defined, afaik #666666 The material color is dynamic, so you'd need to call a theming util function for that. Switching to colorOnSurfaceVariant would force you to change that everywhere in the UI so I wouldn't not do it in this PR but just make the changed one Grey again. |
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
c049058 to
453f0fe
Compare
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
I kept the icon colors as it is (@color/secondary_text_color - #666666). Then we can save it for other PR. I will check your first message. |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/16418.apk |
kra-mo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense from the design side. Let's discuss more on Tuesday :)

Fixes
Share internal link alignment
Dropdown icon visibility (only visible if permission name exists)
Converts image views to material icon button since they are button not icon