Skip to content

Do not switch to root folder if filelist is already shown - #12554

Merged
MorrisJobke merged 1 commit into
masterfrom
bugfix/12493/root-folder-switch
Nov 22, 2018
Merged

Do not switch to root folder if filelist is already shown#12554
MorrisJobke merged 1 commit into
masterfrom
bugfix/12493/root-folder-switch

Conversation

@juliusknorr

@juliusknorrjuliusknorr commented Nov 20, 2018

Copy link
Copy Markdown
Member

Navigating to the root folder is already handled by OCA.Files.Navigation.setActiveItem in case the view doesn't change.

Fixes#12493 since there was a race condition between the root folder loading for falling back when clicking the All files sidebar entry and the actual view that should be loaded.

@juliusknorrjuliusknorr added this to the Nextcloud 15 milestone Nov 20, 2018
@juliusknorr

juliusknorr commented Nov 20, 2018

Copy link
Copy Markdown
MemberAuthor

/backport to stable14

ChristophWurst
ChristophWurst approved these changes Nov 20, 2018
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense

@MorrisJobke

MorrisJobke commented Nov 20, 2018

Copy link
Copy Markdown
Member

JSUnit fails:

PhantomJS 2.1.1 (Linux 0.0.0) OCA.Files.FileList tests loading file list reloads the list when leaving hidden state FAILED
Expected false to equal true.
apps/files/tests/js/filelistSpec.js:1717:41

@MorrisJobkeMorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 20, 2018
@MorrisJobke

MorrisJobke commented Nov 22, 2018

Copy link
Copy Markdown
Member

@juliushaertl Mind to look into this to get the fix shipped in the RC 1

@MorrisJobkeMorrisJobke mentioned this pull request Nov 22, 2018
Merged
7 tasks
@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Nov 22, 2018
@juliusknorr

juliusknorr commented Nov 22, 2018•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@MorrisJobke I'm on it, the test actually catched a bug of this solution, so I'll need to do some adjustments to the fix.

@juliusknorr
Do not switch to root folder if filelist is already shown
a90b4bc
Navigating to the root folder is already handled by
OCA.Files.Navigation.setActiveItem in case the view doesn't change.
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the bugfix/12493/root-folder-switch branch from 2b1d43c to a90b4bcCompareNovember 22, 2018 13:03
@juliusknorr

juliusknorr commented Nov 22, 2018

Copy link
Copy Markdown
MemberAuthor

@MorrisJobke I think I found a way to stop triggering the reload without interfering with existing behaviour. Please give this another test @ChristophWurst@skjnldsv since the logic changed a bit to work around the crazy event passing happening in the files app.

@juliusknorrjuliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 22, 2018
MorrisJobke
MorrisJobke approved these changes Nov 22, 2018
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobkeMorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 22, 2018
JanDragon
JanDragon approved these changes Nov 22, 2018
View reviewed changes

@JanDragonJanDragon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks Good.

@MorrisJobke
MorrisJobke merged commit 2e52ea5 into masterNov 22, 2018
@MorrisJobke
MorrisJobke deleted the bugfix/12493/root-folder-switch branch November 22, 2018 15:10
@MorrisJobke

MorrisJobke commented Nov 22, 2018

Copy link
Copy Markdown
Member

/backport to stable14

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Nov 22, 2018
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Nov 22, 2018

Copy link
Copy Markdown

backport to stable14 in #12593

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@ChristophWurstChristophWurstChristophWurst approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@JanDragonJanDragonJanDragon approved these changes
@rullzerrullzerAwaiting requested review from rullzer
@icewind1991icewind1991Awaiting requested review from icewind1991

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: files

Projects

None yet

Milestone

Nextcloud 15

Development

Successfully merging this pull request may close these issues.

5 participants

@juliusknorr@MorrisJobke@ChristophWurst@skjnldsv@JanDragon