Bring new modal component #3378

LukasHirt · 2020-04-24T10:41:58Z

Related Issue

Refs Dialog box improvements #2263

How Has This Been Tested?

test environment: drone

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Open tasks:

update-docs · 2020-04-24T10:42:02Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

kulmann · 2020-05-13T11:38:57Z

A few things I noticed during local test in Chrome:

sidebar needs to be merged first (see screenshot)
error messages below the input are gone (see screenshot)
delete prompt shows a text input for creating a new folder if that input prompt was opened before (page reload fixes that, so that is might be a keying issue), see screenshot
when entering something into the input from the issue above, hitting cancel in the danger modal triggers the confirm action, so that file/folder gets deleted although I clicked on cancel. this only happens, if something was entered in the input. If the input is unchanged, the modal behaves correctly on cancel.

kulmann · 2020-05-13T11:40:05Z

I added merging the sidebar PR to the list of things to be done in your issue description. Hope you don't mind me editing there.

LukasHirt · 2020-05-14T08:58:11Z

I added merging the sidebar PR to the list of things to be done in your issue description. Hope you don't mind me editing there.

Not at all, thank you!

LukasHirt · 2020-05-14T09:23:12Z

I checked the error message and it seems to work fine for me. The console also not logging any error related to it.

LukasHirt · 2020-05-14T09:26:00Z

when entering something into the input from the issue above, hitting cancel in the danger modal triggers the confirm action, so that file/folder gets deleted although I clicked on cancel. this only happens, if something was entered in the input. If the input is unchanged, the modal behaves correctly on cancel.

That is really weird. Happens even in the other dialogs e.g. create

LukasHirt · 2020-05-14T09:31:51Z

delete prompt shows a text input for creating a new folder if that input prompt was opened before (page reload fixes that, so that is might be a keying issue), see screenshot

Should be fixed now. I didn't properly handle going back to default values in the state.

LukasHirt · 2020-05-25T08:39:06Z

Requires ODS update after merging owncloud/owncloud-design-system#768 and owncloud/owncloud-design-system#769

LukasHirt · 2020-05-26T09:09:56Z

Rebased and ODS updated. The cancel which triggered confirm is fixed now.

LukasHirt · 2020-05-27T12:00:12Z

tests/acceptance/pageObjects/FilesPageElement/filesList.js

-
-      let isPopupVisible = false
-
-      /* issue: https://github.com/owncloud/phoenix/issues/1728


This issue should be fixed so there is no need for this workaround anymore.

LukasHirt · 2020-05-27T22:46:23Z

Hmm, there are still two failing tests. Both failing when comparing the content of a file which has been upload by overwriting the original one. Locally those tests pass though. I'll try some more digging in the morning.

tests/acceptance/stepDefinitions/filesContext.js

LukasHirt · 2020-05-28T11:02:50Z

I cannot figure out why is it failing. The exact same scenario which fails here passes locally. Even tried logging the content of files and it is successfully overwritten. And watching it in VNC viewer also seems that everything is ok, the modal gets displayed and confirmed, the file gets uploaded...

LukasHirt · 2020-05-28T11:03:41Z

@individual-it Could maybe someone from your team take a look at why tests are passing locally but not in CI? I'm probably looking at this already too long and am overlooking something obvious 🤢

kulmann

One thing that I was thinking about, is, how we want to center the modal. At the moment it's centered in the viewport. It could be a good idea to center it in the extension content area (meaning, offset by the width of the left sidebar, if it is visible). Open for discussion, should not be part of this PR imo.

kulmann · 2020-05-28T13:18:24Z

apps/files/src/components/FilesApp.vue

@@ -99,8 +70,7 @@ export default {
      fileName: '',
      selected: [],
      breadcrumbs: [],
-      self: {},
-      renameDialogErrorMessage: null
+      self: {}


Please also remove self here. It is unused and should not be part of data anyway.

kulmann · 2020-05-28T13:41:26Z

apps/files/src/mixins/deleteResources.js

+        ? this.$gettext(
+            'Are you sure you want to delete all selected resources? All their content will be permanently removed. This action cannot be undone.'
+          )
+        : this.$gettext('Are you sure you want to delete all selected resources')


Message misses a question mark at the end

kiranparajuli589 · 2020-05-29T11:31:51Z

@LukasHirt I think the upload is not successful. see last line of browser log at https://drone.owncloud.com/owncloud/phoenix/9884/25/19

9:40:21 AM - SEVERE - http://owncloud/remote.php/dav/public-files/IfKFn6NdAJO3Bbj/lorem.txt - Failed to load resource: the server responded with a status of 500 (Internal Server Error) | 229s
504

Server log at that timestamp:

{"reqId":"ae902097-25e9-492b-b884-708e1cff0cbe",
"level":3,
"time":"2020-05-29T09:40:21+00:00",
"remoteAddr":"172.20.0.3",
"user":"--",
"app":"webdav",
"method":"PUT",
"url":"\/remote.php\/dav\/public-files\/IfKFn6NdAJO3Bbj\/lorem.txt",
"message":"Caused by: {\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\Forbidden\",\"Message\":\"Permission denied to create file\",\"Code\":0,\"Trace\":\"#0 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1091): OCA\\\\DAV\\\\Files\\\\PublicFiles\\\\PublicSharedRootNode->createFile('lorem.txt', Resource id #14)\\n#1 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/CorePlugin.php(504): Sabre\\\\DAV\\\\Server->createFile('public-files\\\/If...', Resource id #14, NULL)\\n#2 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(89): Sabre\\\\DAV\\\\CorePlugin->httpPut(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#3 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(470): Sabre\\\\DAV\\\\Server->emit('method:PUT', Array)\\n#4 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(251): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#5 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/dav\\\/lib\\\/Server.php(329): Sabre\\\\DAV\\\\Server->start()\\n#6 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#7 \\\/var\\\/www\\\/owncloud\\\/server\\\/remote.php(165): require_once('\\\/var\\\/www\\\/ownclo...')\\n#8 {main}\",\"File\":\"\\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/dav\\\/lib\\\/Files\\\/PublicFiles\\\/PublicSharedRootNode.php\",\"Line\":127}"}

PS: phoenix is setup in CI using bridge https://owncloud.github.io/ocis/bridge/, that might be the reason of different behavior in CI and local setup.

tests/acceptance/pageObjects/filesPage.js

LukasHirt added the Status:In-Progress label Apr 24, 2020

LukasHirt self-assigned this Apr 24, 2020

LukasHirt force-pushed the feature/bring-new-modal branch 4 times, most recently from a9ae7f5 to 0b53edb Compare May 13, 2020 10:24

LukasHirt requested review from kulmann and PVince81 May 13, 2020 10:24

LukasHirt added Category:Enhancement Add new functionality and removed Status:In-Progress labels May 13, 2020

LukasHirt marked this pull request as ready for review May 13, 2020 10:24

LukasHirt force-pushed the feature/bring-new-modal branch from 0b53edb to dde11b0 Compare May 13, 2020 11:33

LukasHirt force-pushed the feature/bring-new-modal branch from dde11b0 to e84a21f Compare May 14, 2020 09:31

LukasHirt force-pushed the feature/bring-new-modal branch from e84a21f to 36b4d5c Compare May 26, 2020 09:08

LukasHirt force-pushed the feature/bring-new-modal branch from 36b4d5c to 1956029 Compare May 27, 2020 11:56

LukasHirt mentioned this pull request May 27, 2020

occasionally file action confirmation dialog get stuck #1728

Closed

LukasHirt commented May 27, 2020

View reviewed changes

LukasHirt force-pushed the feature/bring-new-modal branch from 1956029 to 80dfa52 Compare May 27, 2020 20:37

LukasHirt force-pushed the feature/bring-new-modal branch from 80dfa52 to eba6964 Compare May 27, 2020 21:30

individual-it reviewed May 28, 2020

View reviewed changes

tests/acceptance/stepDefinitions/filesContext.js Show resolved Hide resolved

LukasHirt force-pushed the feature/bring-new-modal branch 2 times, most recently from 0834ff7 to df0b004 Compare May 28, 2020 08:53

Bring new modal component

7f2933d

LukasHirt force-pushed the feature/bring-new-modal branch from df0b004 to 7f2933d Compare May 28, 2020 08:54

kulmann approved these changes May 28, 2020

View reviewed changes

individual-it added the QA:team label May 29, 2020

kiranparajuli589 self-assigned this May 29, 2020

Added ajax call wait after confirm file overwrite

bcbf5c6

kiranparajuli589 force-pushed the feature/bring-new-modal branch from 4b31030 to bcbf5c6 Compare June 1, 2020 11:03

kiranparajuli589 reviewed Jun 1, 2020

View reviewed changes

tests/acceptance/pageObjects/filesPage.js Show resolved Hide resolved

kiranparajuli589 removed their assignment Jun 1, 2020

kiranparajuli589 merged commit 20c4c98 into master Jun 2, 2020

delete-merged-branch bot deleted the feature/bring-new-modal branch June 2, 2020 03:29

LukasHirt mentioned this pull request Jun 2, 2020

Add missing question mark in trashbin delete dialog #3566

Merged

pascalwengerter mentioned this pull request Jun 15, 2021

Dialog box improvements #2263

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bring new modal component #3378

Bring new modal component #3378

LukasHirt commented Apr 24, 2020 •

edited

Loading

update-docs bot commented Apr 24, 2020

kulmann commented May 13, 2020

kulmann commented May 13, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 25, 2020

LukasHirt commented May 26, 2020

LukasHirt May 27, 2020

LukasHirt commented May 27, 2020

LukasHirt commented May 28, 2020

LukasHirt commented May 28, 2020 •

edited

Loading

kulmann left a comment

kulmann May 28, 2020

kulmann May 28, 2020

kiranparajuli589 commented May 29, 2020 •

edited

Loading


		let isPopupVisible = false

		/* issue: https://github.com/owncloud/phoenix/issues/1728

Bring new modal component #3378

Bring new modal component #3378

Conversation

LukasHirt commented Apr 24, 2020 • edited Loading

Related Issue

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Open tasks:

update-docs bot commented Apr 24, 2020

kulmann commented May 13, 2020

kulmann commented May 13, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 14, 2020

LukasHirt commented May 25, 2020

LukasHirt commented May 26, 2020

LukasHirt May 27, 2020

Choose a reason for hiding this comment

LukasHirt commented May 27, 2020

LukasHirt commented May 28, 2020

LukasHirt commented May 28, 2020 • edited Loading

kulmann left a comment

Choose a reason for hiding this comment

kulmann May 28, 2020

Choose a reason for hiding this comment

kulmann May 28, 2020

Choose a reason for hiding this comment

kiranparajuli589 commented May 29, 2020 • edited Loading

LukasHirt commented Apr 24, 2020 •

edited

Loading

LukasHirt commented May 28, 2020 •

edited

Loading

kiranparajuli589 commented May 29, 2020 •

edited

Loading