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

Include more sharing links #363

Merged
merged 1 commit into from
May 8, 2020
Merged

Include more sharing links #363

merged 1 commit into from
May 8, 2020

Conversation

jotoeri
Copy link
Member

@jotoeri jotoeri commented May 7, 2020

Fixes

  • Not clear how to get share link, needs to be in sidebar next to radio option too, and ideally in responses view too when empty
  • Included a link on EmptyContent in results
  • Included a link in Sidebar
  • Changed the Clipboard-package to fix closing of Actions-Menu, when copying link (now remains open, showing the nice green checkmark). Found that package here

grafik
grafik
grafik

@jotoeri jotoeri added bug Something isn't working 3. to review Waiting for reviews feature: 👥 sharing settings labels May 7, 2020
@skjnldsv
Copy link
Member

skjnldsv commented May 7, 2020

Ah, please use the Actions menu on the sidebar then :)
It's dedicated for those uses exactly 😉
https://nextcloud-vue-components.netlify.app/#/Components/App%20containers?id=appsidebar-1

	<AppSidebar>
	...
		<template #secondary-actions>
			<ActionButton>
				{{ t('forms', 'Copy to clipboard') }}
			</ActionButton>
		</template>
	</AppSidebar>

@jancborchardt
Copy link
Member

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".)

@jancborchardt jancborchardt added this to the 2.0 milestone May 7, 2020
@jotoeri
Copy link
Member Author

jotoeri commented May 7, 2020

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.
But yes, for when notifications are running, it will be good to do sharing like this.

Copy link
Member

@jancborchardt jancborchardt left a 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. :)

@jotoeri
Copy link
Member Author

jotoeri commented May 7, 2020

This is when i use the secondary-actions-slot. (Less visible...?)
@jancborchardt Different design works for you?

grafik

@jancborchardt
Copy link
Member

jancborchardt commented May 7, 2020

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.

@jotoeri
Copy link
Member Author

jotoeri commented May 7, 2020

Well i didn't finish the work & commit yet. This is why i ask.
Alternative would be to merge as it was above

@skjnldsv
Copy link
Member

skjnldsv commented May 7, 2020

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.

I'd guess we should also think about others app, Talk is also having the copy to clipboard there?

@skjnldsv
Copy link
Member

skjnldsv commented May 7, 2020

This is when i use the secondary-actions-slot. (Less visible...?)
@jancborchardt Different design works for you?

grafik

Looks good to me :)

Copy link
Member

@skjnldsv skjnldsv left a 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.

@jotoeri
Copy link
Member Author

jotoeri commented May 7, 2020

grafik
grafik

@codecov-io

This comment has been minimized.

Copy link
Member

@skjnldsv skjnldsv left a 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>
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good! :)

@jancborchardt jancborchardt merged commit 3e040c2 into master May 8, 2020
@jancborchardt jancborchardt deleted the fix/copy_link branch May 8, 2020 00:48
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 bug Something isn't working feature: 👥 sharing settings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants