-
Notifications
You must be signed in to change notification settings - Fork 98
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
Include more sharing links #363
Conversation
Ah, please use the Actions menu on the sidebar then :)
|
The "Copy share link" button should only show when the "Share link" radio button is selected, as otherwise it is not active, no? (And in that case it should also show next to that radio option, in the same line. (Sidenote: German translation could be improved if you are on Transifex, shouldn’t say "Öffentlich" as it’s not public – maybe something like "Link teilen".) |
As for now, users don't get a message, if the form is shared to them. So the link is still necessary on all sharing-options. |
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.
In that case 👍 design-wise, but please see @skjnldsv’s remark. :)
This is when i use the secondary-actions-slot. (Less visible...?) |
Hmm, since this should be the main way to get the share link, I’m not sure about it. Let’s get it in for now since the pull request improves it a lot, but it being icon-only and separated from the actual "Sharing" settings below, there will be issues. Small edit: It should say what it will copy. That is: "Copy share link" – mentioning "clipboard" is not necessary. |
Well i didn't finish the work & commit yet. This is why i ask. |
I'd guess we should also think about others app, Talk is also having the copy to clipboard there? |
Looks good to me :) |
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.
Ah, sorry! I forgot to mention!
it should be ActionLinks and links, not button.
So you can copy or open in a new tab too.
This comment has been minimized.
This comment has been minimized.
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.
Code looks good! :)
Signed-off-by: Jonas Rittershofer <jotoeri@users.noreply.github.com>
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.
Looks good! :)
Fixes