Skip to content

Conversation

@danxuliu
Copy link
Member

When the sidebar is not available sidebarFile is null. As the condition strictly checked for an empty string it passed when not empty but also when null, so the sidebar was tried to be opened even if it was not available.

Another possible fix would have been to change the computed property to return an empty string too if the sidebar is not available. However I changed the condition instead as I thought that it would be better to be able to differentiate between those states (even if it is currently not needed). Having said that, if you prefer that approach just tell me and I will change it ;-)

@danxuliu danxuliu added bug Something isn't working 3. to review Waiting for reviews javascript Javascript related ticket labels Mar 10, 2020
@danxuliu danxuliu requested a review from skjnldsv March 10, 2020 15:01
@cypress
Copy link

cypress bot commented Mar 10, 2020



Test summary

135 0 0 0


Run details

Project viewer
Status Passed
Commit 6b40267 ℹ️
Started Mar 12, 2020 4:55 PM
Ended Mar 12, 2020 4:57 PM
Duration 01:40 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 78

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@skjnldsv
Copy link
Member

@danxuliu
Copy link
Member Author

Already here: https://github.com/nextcloud/viewer/pull/408/files#diff-8221959d5bec03dbe64fc1a567c39719L402

Unfortunately that fails if OCA.Files is not defined, as it tries to access the Sidebar property of undefined :-(

I can rebase this pull request on latest master to fix it. Which condition do you prefer, if (this.sidebarFile) { or if (this.Sidebar && this.sidebarFile !== '') {?

@skjnldsv
Copy link
Member

Yours is fine :)

When OCA.Files is not defined the sidebar is not available, but in that
case it is not possible to check against OCA.Files.Sidebar.

When the sidebar is not available "sidebarFile" is null, and when the
sidebar is available but there is no file it is an empty string, so it
is enough to check if "sidebarFile" has a value or not in order to open
the sidebar or not.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the fix-trying-to-open-the-sidebar-when-not-available branch from 25aecb3 to f3d1fd0 Compare March 12, 2020 16:53
@skjnldsv skjnldsv merged commit ffedd7a into master Mar 18, 2020
@skjnldsv skjnldsv deleted the fix-trying-to-open-the-sidebar-when-not-available branch March 18, 2020 19:38
@skjnldsv
Copy link
Member

/backport to stable18

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Mar 18, 2020
@backportbot-nextcloud
Copy link

The backport to stable18 failed. Please do this backport manually.

@skjnldsv skjnldsv removed the backport-request Pending backport by the backport-bot label Mar 19, 2020
@skjnldsv
Copy link
Member

backport in #428

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 javascript Javascript related ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants