-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Fix sharing icon #42502
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
Fix sharing icon #42502
Conversation
|
Seems like the first icon on your screenshot isn't what is being discussed on the mentioned issue: #40202 |
| <template #icon> | ||
| <LinkIcon v-if="shareButtonType === Type.SHARE_TYPE_LINK" /> | ||
| <ShareVariantIcon v-else :size="20" /> | ||
| <FolderAccountIcon v-if="shareButtonType !== null" /> |
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.
@jancborchardt shall we lose the share link icon too?
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.
If it’s shared by link, that should be reflected?
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.
I agree, but this PR removes that
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.
@jancborchardt we said in yesterday's design review that we're not going to use this icon in iOS in favor of the native one. I think that in line with that, we should use the material folder-shared icon that @szaimen proposed in the issue both in the web interface and in the android apps.
I see now that @jancborchardt refers to the folder with the link in it. I think that the folder-share material design icon is more appropriate, both here and in the files list. |
|
@marcoambrosini @skjnldsv let’s indeed focus on the issue at hand here to prevent creating deadlocks like these. :) This icon https://fonts.google.com/icons?selected=Material%20Icons%3Aperson_add_alt_1%3A communicates both "Share" and "Add to share", so it can be used in both unshared and shared state, which we currently do not separate in the breadcrumbs either. |
| type="tertiary" | ||
| @click="openSharingSidebar"> | ||
| <template #icon> | ||
| <LinkIcon v-if="shareButtonType === Type.SHARE_TYPE_LINK" /> |
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.
this should probably be restored?
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.
The issue at hand proposes the usage of this new icon @jancborchardt
We currently have separate states in the breadcrumbs icon, just with a worse logic. A different icon is shown only if the folder is shared by link. No distinction is made between unshared and shared without link. |
|
So as @skjnldsv said, we don’t use the proposed icon anywhere. What we do use is these ones: The core issue is mainly that in the breadcrumbs we still use the "triangle" share icon, and that needs to be replaced. If we discuss more than that then it will of course lead to unnecessary discussion and block a simple fix as this → so let’s keep the matters separate. |
|
Moving assignment from @marcoambrosini to @szaimen as discussed in the design team call. :) |
|
Superceded by #43433 |



Summary
shared state
unshared state
TODO
Checklist