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.

Sign up for GitHub

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

Jump to bottom

Dont show menu automatically when share link is clicked #16454

Merged
merged 2 commits into from
Sep 4, 2019
Merged

Dont show menu automatically when share link is clicked #16454

merged 2 commits into from
Sep 4, 2019

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Jul 18, 2019

ref #16076

Share link should not open menu automatically

@GretaD GretaD requested a review from georgehrke July 18, 2019 12:30
@GretaD GretaD self-assigned this Jul 18, 2019
@GretaD GretaD added 3. to review Waiting for reviews bug labels Jul 18, 2019
@georgehrke
Copy link
Member

/compile fixup /

@juliushaertl
Copy link
Member

This is basically what #14280 was supposed to do for links that don't require additional options. @jancborchardt Wasn't to happy about generally not showing the menu:

Don’t you think it’s useful to open the menu (or at least for the first link of a file)? It is only shown when relevant, when you create the link. People who are not familiar with Nextcloud (= most people) will discover the settings directly then.

@jancborchardt Maybe we should try to get it in as a temporary fix for 17 until #276 is done with 18?

@juliushaertl
Copy link
Member

If we do it, acceptance tests will need to be adjusted 😉

@jancborchardt
Copy link
Member

jancborchardt commented Jul 18, 2019 •
edited
Loading

Yeah, but this came up in the polishing session, and opening the menu every time is quite annoying now that we used it for some time.

We could still add an enhancement that opens the menu only once per user. But we definitely should merge this here as papercut.

@georgehrke
Copy link
Member

@juliushaertl Can you give @GretaD some hints about the integration tests? :)

@juliushaertl
Copy link
Member

juliushaertl commented Jul 18, 2019 •
edited
Loading

@GretaD For running them locally have a look at the run script https://github.com/nextcloud/server/blob/master/tests/acceptance/run.sh#L20 which also has some nice writeup by @danxuliu Note that it will require you to have docker setup on your local system.

This is probably the test that needs adjustment:

server/tests/acceptance/features/app-files-sharing-link.feature

Line 12 in 0968343

Then I see that the Share menu is shown

@GretaD @danxuliu
Dont show menu automatically when share link is clicked
53aa73d 
Signed-off-by: Greta Doci <gretadoci@gmail.com>
@GretaD @danxuliu
Adjust acceptance tests to changes in link share menu behaviour
1993984 
Now the link share menu is not automatically opened after a link share
is created, so waiting until it was opened failed in iShareTheLinkFor.

Note that the steps that interact with the link share menu take care
themselves of showing the menu if needed, so there is no need to
explicitly show it despite the change. Also, the waiting in
iShareTheLinkFor was introduced when the link share menu was changed
to automatically open after creating a link share, as that caused some
issues with the steps that opened the menu by themselves (fec8d12).
Due to all this, now that the link share menu is again not automatically
opened the wait can be simply removed.

Signed-off-by: Greta Doci <gretadoci@gmail.com>
@danxuliu danxuliu force-pushed the bugfix/noid/sharelink-shouldnt-open-menu-automatically branch from 8464ccd to 1993984 Compare August 29, 2019 09:01
@danxuliu
Copy link
Member

Rebased and fixed up

danxuliu
danxuliu approved these changes Aug 29, 2019
View reviewed changes
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@danxuliu danxuliu added this to the Nextcloud 17 milestone Aug 29, 2019
@rullzer rullzer mentioned this pull request Aug 29, 2019
Merged
16 tasks
@rullzer rullzer merged commit 2fdf946 into master Sep 4, 2019
@rullzer rullzer deleted the bugfix/noid/sharelink-shouldnt-open-menu-automatically branch September 4, 2019 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@georgehrke georgehrke georgehrke approved these changes

@danxuliu danxuliu danxuliu approved these changes

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

Assignees

@GretaD GretaD

Labels
3. to review Waiting for reviews bug
Projects
None yet
Milestone
Nextcloud 17
Development

Successfully merging this pull request may close these issues.
6 participants
@GretaD @georgehrke @juliushaertl @jancborchardt @danxuliu @rullzer