Skip to content

Change sidebar file while changing file in slideshow - #394

Merged
skjnldsv merged 1 commit into
masterfrom
enh/380/change-sidebar-on-image-change
Feb 25, 2020
Merged

Change sidebar file while changing file in slideshow#394
skjnldsv merged 1 commit into
masterfrom
enh/380/change-sidebar-on-image-change

Conversation

@gary-kim

Copy link
Copy Markdown
Member

Closes#380

Change sidebar file when changing file in slideshow.
Also made it so that the sidebar gets opened again when opening the viewer so that the sidebar does not end up under the modal.

Signed-off-by: Gary Kim gary@garykim.dev

@gary-kimgary-kim added enhancement New feature or request 3. to review Waiting for reviews labels Feb 10, 2020
@gary-kimgary-kim added this to the Nextcloud 19 milestone Feb 10, 2020
@gary-kim
gary-kimforce-pushed the enh/380/change-sidebar-on-image-change branch 2 times, most recently from c726965 to b931e2eCompareFebruary 11, 2020 13:20
@skjnldsv

Copy link
Copy Markdown
Member

Could you add tests please? :)

@skjnldsvskjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 24, 2020
@gary-kim
gary-kimforce-pushed the enh/380/change-sidebar-on-image-change branch 3 times, most recently from 784ac6b to 59803cbCompareFebruary 25, 2020 03:54
@cypress

cypressBot commented Feb 25, 2020

Copy link
Copy Markdown



Test summary

135000


Run details

Projectviewer
StatusPassed
Commitad5cd2eℹ️
StartedFeb 25, 2020 2:27 PM
EndedFeb 25, 2020 2:28 PM
Duration01:51 💡
OSLinux Ubuntu Linux - 18.04
BrowserElectron 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

@gary-kim
gary-kimforce-pushed the enh/380/change-sidebar-on-image-change branch 5 times, most recently from 3e1fdda to dc5c5f5CompareFebruary 25, 2020 05:19
@gary-kimgary-kim added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 25, 2020
@gary-kim

Copy link
Copy Markdown
MemberAuthor

Could you add tests please? :)

Tests added for both sidebar image changing and opening viewer with sidebar open.

Comment threadcypress/integration/sidebar.spec.js
@skjnldsv

Copy link
Copy Markdown
Member

/backport to stable18

@backportbot-nextcloudbackportbot-nextcloudBot added the backport-request Pending backport by the backport-bot label Feb 25, 2020
@gary-kim
gary-kimforce-pushed the enh/380/change-sidebar-on-image-change branch from dc5c5f5 to 904a94cCompareFebruary 25, 2020 12:15
@cypress

cypressBot commented Feb 25, 2020

Copy link
Copy Markdown



Test summary

134000


Run details

Projectviewer
StatusPassed
Commit904a94c
StartedFeb 25, 2020 12:27 PM
EndedFeb 25, 2020 12:29 PM
Duration01:38 💡
OSLinux Ubuntu Linux - 18.04
BrowserElectron 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

Comment threadcypress/integration/sidebar.spec.js
Signed-off-by: Gary Kim <gary@garykim.dev>
@gary-kim
gary-kimforce-pushed the enh/380/change-sidebar-on-image-change branch from 904a94c to 6fc9760CompareFebruary 25, 2020 14:24
@gary-kim

Copy link
Copy Markdown
MemberAuthor

Open the sidebar -- Does not have any visual regression 4 diff
🤦‍♂️

@skjnldsv

Copy link
Copy Markdown
Member

Yes, do no commit the screenshots ;)

@skjnldsv

Copy link
Copy Markdown
Member

Great work :)
One question though, what is expected on the last step Opening the sidebar before the viewer? I don't see any relevant actions on the cypress run 🤔

@gary-kim

gary-kim commented Feb 25, 2020

Copy link
Copy Markdown
MemberAuthor

One question though, what is expected on the last step Opening the sidebar before the viewer? I don't see any relevant actions on the cypress run thinking

Checking that the sidebar does not end up under the modal when the viewer is opened with the sidebar already being opened.

@skjnldsv

Copy link
Copy Markdown
Member

Make sense :)

@skjnldsv
skjnldsv merged commit c54661f into masterFeb 25, 2020
@skjnldsv
skjnldsv deleted the enh/380/change-sidebar-on-image-change branch February 25, 2020 15:25
@backportbot-nextcloud

Copy link
Copy Markdown

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

@gary-kim

Copy link
Copy Markdown
MemberAuthor

Backport in #405

@gary-kimgary-kim added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews backport-request Pending backport by the backport-bot labels Feb 26, 2020
Comment threadsrc/views/Viewer.vue
* Show sidebar if sidebar is already opened
*/
changeSidebar() {
if (this.sidebarFile !== '') {

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.

Eheh, this doesn't check against null or undefined.
Meaning if the sidebar is not available (no OCA.Files.Sidebar) it will still try to open :)

I'll fix in a secondary pr

susnux pushed a commit that referenced this pull request May 16, 2026
…e-change
Change sidebar file while changing file in slideshow
susnux pushed a commit that referenced this pull request May 16, 2026
…e-change
Change sidebar file while changing file in slideshow
susnux pushed a commit that referenced this pull request May 16, 2026
…e-change
Change sidebar file while changing file in slideshow
susnux pushed a commit that referenced this pull request May 16, 2026
…e-change
Change sidebar file while changing file in slideshow
susnux pushed a commit that referenced this pull request May 16, 2026
…e-change
Change sidebar file while changing file in slideshow
susnux pushed a commit that referenced this pull request May 16, 2026
…e-change
Change sidebar file while changing file in slideshow
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change sidebar file while changing file in slideshow

2 participants

@gary-kim@skjnldsv