Move sharing page options to menu in top right #6652

jancborchardt · 2017-09-26T15:16:47Z

Making the sharing page a bit cleaner. Before & after:

TO DO:

Make the dropdown interactive – need @nextcloud/javascript help for this
~~Clicking »Direct link« should copy it to clipboard and give that as feedback in a tooltip~~
When there is no preview but only filetype icon (like for zip files etc), the download button should show directly under the filetype icon too

skjnldsv · 2017-09-26T15:40:26Z

Nice pr! :)

Make the dropdown interactive

What do you mean?

codecov · 2017-09-26T15:41:36Z

Codecov Report

❗ No coverage uploaded for pull request base (master@ce12cd9). Click here to learn what that means.
The diff coverage is 21.87%.

@@            Coverage Diff            @@
##             master    #6652   +/-   ##
=========================================
  Coverage          ?   50.61%           
  Complexity        ?    24297           
=========================================
  Files             ?     1577           
  Lines             ?    92922           
  Branches          ?     1359           
=========================================
  Hits              ?    47036           
  Misses            ?    45886           
  Partials          ?        0

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/templates/public.php	`0% <0%> (ø)`	`0 <0> (?)`
apps/files_sharing/js/public.js	`48.74% <63.63%> (ø)`	`0 <0> (?)`

jancborchardt · 2017-09-26T15:44:09Z

@skjnldsv I mean not showing that dropdown permanently, and only opening it on click of the Download action. :)

skjnldsv · 2017-09-26T15:51:00Z

Oh ok! :)

$(document).ready(function() {
	$('.app-navigation-entry-utils-menu-button button:not([class])').click(function() {
		$(this).parents('.app-navigation-entry-utils').next().show();
	});
})
$(document).mouseup(function(e) {
	var container = $(".app-navigation-entry-menu");

	// if the target of the click isn't the container nor a descendant of the container
	if (!container.is(e.target) && container.has(e.target).length === 0) {
		container.hide();
	}
});

It's what I use on the simple template app I made, not sure if this is a good idea though :)
We definitely should implement this into master!

jancborchardt · 2017-09-29T16:03:27Z

@skjnldsv added it, thanks! :) I tried to play around with .toggle() to also close the menu back on second click but it seems to not work. Any idea?

jancborchardt · 2017-09-29T16:05:35Z

Also I might have used a really rough way to decide if the download button should be shown under the filetype icon too – for files which can’t be previewed, i.e. anything except images: https://github.com/nextcloud/server/pull/6652/files#diff-28509333ad5b898bf1af427bd3e64113R103 cc @rullzer

MorrisJobke · 2017-10-23T16:14:36Z

🏓 for review here :)

jancborchardt · 2017-10-24T13:08:43Z

Yes, please review @nextcloud/designers @nextcloud/javascript :)

skjnldsv · 2017-10-24T13:15:20Z

Everything seems good with this! But before I review, can anyone tell me what the "direct link" option is supposed to do?

jancborchardt · 2017-10-24T13:27:16Z

Direct link for images is supposed to only show the image (without background or header) so it can be embedded directly.

For filetypes other than files which can directly be shown, it's basically just a link to the page (cause lots of regular people don't know a URL from the bar can be copied, and look for a "Copy link" option).

skjnldsv

Sorry there is so many changes needed 🙈
This is looking great nonetheless, much easier to read on mobile/narrow screens. 😅

But it makes me wonder if we shouldn't display as many button as we could and but them in the menu if there isn't enough space left, same as the app list on the nextcloud header. :)

skjnldsv · 2017-10-24T13:31:18Z

apps/files_sharing/templates/public.php

-					<span id="download-text"><?php p($l->t('Download'))?></span>
-				</a>
+			<?php if (!isset($_['hideFileList']) || (isset($_['hideFileList']) && $_['hideFileList'] === false)) { ?>
+			<a href="#" id="share-menutoggle" class="menutoggle icon-more-white"><span class="share-menutoggle-text"><?php p($l->t('Download')) ?></span></a>


Why is the download link having the icon-more-white class? This needs to be 2 separate elements. :)

Download is just the text label for the menu, so it's the same element. :)

It feels strange! :/ :)

skjnldsv · 2017-10-24T13:33:31Z

apps/files_sharing/templates/public.php

-				</a>
+			<?php if (!isset($_['hideFileList']) || (isset($_['hideFileList']) && $_['hideFileList'] === false)) { ?>
+			<a href="#" id="share-menutoggle" class="menutoggle icon-more-white"><span class="share-menutoggle-text"><?php p($l->t('Download')) ?></span></a>
+			<div id="share-menu" class="popovermenu menu hidden">


You do not need the hidden class as the popover is hidden by default.

skjnldsv · 2017-10-24T13:33:47Z

apps/files_sharing/js/public.js

@@ -424,4 +424,18 @@ $(document).ready(function () {
 			return App.fileList.generatePreviewUrl(urlSpec);
 		};
 	}
+
+	$('#share-menutoggle').click(function() {
+		$('#share-menu').show();


Please use the class 'open'.

That's the code you gave me above. ;)

skjnldsv · 2017-10-24T13:33:52Z

apps/files_sharing/js/public.js

+
+	// if the target of the click isn't the container nor a descendant of the container
+	if (!container.is(e.target) && container.has(e.target).length === 0) {
+		container.hide();


Please use the class 'open'.

skjnldsv · 2017-10-24T13:35:05Z

apps/files_sharing/templates/public.php

@@ -84,16 +100,14 @@
 					<!-- Preview frame is filled via JS to support SVG images for modern browsers -->
 					<div id="imgframe"></div>
 				<?php endif; ?>
+				<?php if ($_['previewURL'] === $_['downloadURL']): ?>
 				<div class="directDownload">
 					<a href="<?php p($_['downloadURL']); ?>" id="downloadFile" class="button">


It's not working on my setup, No response or feedback when clicking this link.
Maybe we should also fix this here?

skjnldsv · 2017-10-24T13:35:41Z

core/css/header.scss

@@ -143,6 +143,8 @@
 	#header-left, .header-left {


This could have a display: inline-flex for easier alignment .

That’s already set in the lines above. :)

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Fixed

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

jancborchardt · 2017-11-01T13:51:03Z

This is now ready for review @nextcloud/javascript @nextcloud/designers @nextcloud/sharing! :)

jancborchardt · 2017-11-02T09:52:23Z

@skjnldsv can you re-review? :) @rullzer @schiessle @MorrisJobke can you also review please?

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

rullzer

Seems to work!
Lets get this in!

jancborchardt added 2. developing Work in progress design Design, UI, UX, etc. enhancement feature: sharing help wanted labels Sep 26, 2017

jancborchardt added this to the Nextcloud 13 milestone Sep 26, 2017

jancborchardt requested review from MorrisJobke, schiessle, pixelipo and skjnldsv September 26, 2017 15:16

jancborchardt force-pushed the sharing-link-menu branch from c1ddb08 to a42f62b Compare September 29, 2017 15:49

jancborchardt added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 29, 2017

jancborchardt requested review from rullzer and ChristophWurst September 29, 2017 16:03

skjnldsv previously requested changes Oct 24, 2017

View reviewed changes

jancborchardt added 4 commits November 1, 2017 11:52

Move shared page controls to menu in top right

3d05540

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Fix header text hiding completely or wrapping wrongly

93d14dd

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Fix display of Download entry text

40a3b01

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Add label to download action

9f4cd52

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

jancborchardt added 4 commits November 1, 2017 11:52

Adjust prominence of shared-by info to not overpower file name

f2fadf6

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Show and hide menu via Javascript, thanks @skjnldsv

116cb58

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Fix scrollbar showing up on public page

b3ccaf8

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Use standard way of showing and hiding popover menu

bc47668

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

jancborchardt force-pushed the sharing-link-menu branch from d3a53ea to bc47668 Compare November 1, 2017 11:10

jancborchardt force-pushed the sharing-link-menu branch from ab45dd8 to 905b3c5 Compare November 1, 2017 13:15

Directly show link

18dff92

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

jancborchardt force-pushed the sharing-link-menu branch from 905b3c5 to 18dff92 Compare November 1, 2017 13:17

Clicking publicly shared image opens direct preview url

84b8cb1

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

skjnldsv approved these changes Nov 2, 2017

View reviewed changes

Fix Scrutinizer issue

eebf19e

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

rullzer approved these changes Nov 2, 2017

View reviewed changes

rullzer added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews help wanted labels Nov 2, 2017

skjnldsv merged commit 3091ddb into master Nov 2, 2017

skjnldsv deleted the sharing-link-menu branch November 2, 2017 16:41

danxuliu mentioned this pull request Dec 21, 2017

Fix closing menu on second click in sharing page #7598

Merged

danxuliu mentioned this pull request Jan 8, 2018

Sharing page header should follow the new layout introduced in Nextcloud 13 nextcloud/gallery#358

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move sharing page options to menu in top right #6652

Move sharing page options to menu in top right #6652

jancborchardt commented Sep 26, 2017 •

edited

Loading

skjnldsv commented Sep 26, 2017 •

edited

Loading

codecov bot commented Sep 26, 2017 •

edited

Loading

jancborchardt commented Sep 26, 2017

skjnldsv commented Sep 26, 2017 •

edited

Loading

jancborchardt commented Sep 29, 2017

jancborchardt commented Sep 29, 2017

MorrisJobke commented Oct 23, 2017

jancborchardt commented Oct 24, 2017

skjnldsv commented Oct 24, 2017

jancborchardt commented Oct 24, 2017

skjnldsv left a comment •

edited

Loading

skjnldsv Oct 24, 2017

jancborchardt Oct 27, 2017

skjnldsv Oct 27, 2017

skjnldsv Oct 24, 2017

skjnldsv Oct 24, 2017

jancborchardt Oct 27, 2017

skjnldsv Oct 24, 2017

skjnldsv Oct 24, 2017

skjnldsv Oct 24, 2017

jancborchardt Nov 1, 2017

jancborchardt commented Nov 1, 2017

jancborchardt commented Nov 2, 2017

rullzer left a comment

Move sharing page options to menu in top right #6652

Move sharing page options to menu in top right #6652

Conversation

jancborchardt commented Sep 26, 2017 • edited Loading

skjnldsv commented Sep 26, 2017 • edited Loading

codecov bot commented Sep 26, 2017 • edited Loading

Codecov Report

jancborchardt commented Sep 26, 2017

skjnldsv commented Sep 26, 2017 • edited Loading

jancborchardt commented Sep 29, 2017

jancborchardt commented Sep 29, 2017

MorrisJobke commented Oct 23, 2017

jancborchardt commented Oct 24, 2017

skjnldsv commented Oct 24, 2017

jancborchardt commented Oct 24, 2017

skjnldsv left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jancborchardt commented Nov 1, 2017

jancborchardt commented Nov 2, 2017

rullzer left a comment

Choose a reason for hiding this comment

jancborchardt commented Sep 26, 2017 •

edited

Loading

skjnldsv commented Sep 26, 2017 •

edited

Loading

codecov bot commented Sep 26, 2017 •

edited

Loading

skjnldsv commented Sep 26, 2017 •

edited

Loading

skjnldsv left a comment •

edited

Loading