Skip to content

Conversation

@georgehrke
Copy link
Member

@georgehrke georgehrke commented Jul 16, 2020

Signed-off-by: Georg Ehrke [email protected]

Breaking changes:

  • #app-content to .app-content
  • #app-navigation to .app-navigation
  • #app-sidebar to .app-sidebar
  • #content to .content
  • Navigation List Slot
  • AppNavigationSettings Footer Slot

@georgehrke georgehrke added the 2. developing Work in progress label Jul 16, 2020
@georgehrke
Copy link
Member Author

/compile /

</div>
</AppNavigationSettings>
</template>
<template #footer>
Copy link
Member

Choose a reason for hiding this comment

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

Wait, why did we need a footer slot? I completely missed this! 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Interesting choice, I would have put the default slot to be the list and not the Navigation new button 🤷

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Thanks for this @georgehrke ❤️

@skjnldsv
Copy link
Member

Needs big rebase.

@georgehrke georgehrke force-pushed the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch from 84a3da7 to 303fedb Compare July 17, 2020 07:05
@georgehrke
Copy link
Member Author

Rebased

@georgehrke georgehrke requested a review from rullzer July 17, 2020 11:20
@georgehrke georgehrke mentioned this pull request Jul 17, 2020
3 tasks
@ChristophWurst
Copy link
Member

dependageorg

Nice

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Not sure if ready to review but the code looks fine :)

@georgehrke georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 17, 2020
@skjnldsv
Copy link
Member

Acceptance apps/users fails & jsunit too.

@georgehrke
Copy link
Member Author

PhantomJS 2.1.1 (Linux 0.0.0) DEBUG: 'OCA.Files.Settings initialized'
1011	PhantomJS 2.1.1 (Linux 0.0.0) ERROR
1012	  {
1013	    "message": "SyntaxError: Unexpected token 'const'\nat apps/files/js/dist/personal-settings.js:1:0",
1014	    "str": "SyntaxError: Unexpected token 'const'\nat apps/files/js/dist/personal-settings.js:1:0"
1015	  }
1016	PhantomJS 2.1.1 (Linux 0.0.0): Executed 0 of 0 ERROR (0.374 secs / 0 secs)
1017	PhantomJS 2.1.1 (Linux 0.0.0) ERROR
1018	  {
1019	    "message": "SyntaxError: Unexpected token 'const'\nat apps/files/js/dist/sidebar.js:1:0",
1020	    "str": "SyntaxError: Unexpected token 'const'\nat apps/files/js/dist/sidebar.js:1:0"
1021	  }

@skjnldsv
Copy link
Member

I guess this is the same issue I reported recently.
There is an issue loading the scripts on files on master 🤔

@georgehrke georgehrke force-pushed the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch 2 times, most recently from d5406ba to 571bb49 Compare July 20, 2020 08:54
@georgehrke
Copy link
Member Author

I guess this is the same issue I reported recently.
There is an issue loading the scripts on files on master 🤔

The nc/vue bundle seems to be using const in the built version. I added it to the webpack config.

@skjnldsv skjnldsv added enhancement feature: dependencies high javascript 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 20, 2020
@skjnldsv
Copy link
Member

Transpile @nc/vue and some of its dependencies 2e6775b

I'm not sure abou tthis one! Don't we already transpile on the library?
https://github.com/nextcloud/nextcloud-vue/blob/master/babel.config.js

@georgehrke georgehrke force-pushed the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch from 10c79d8 to 0698123 Compare July 20, 2020 10:54
@georgehrke georgehrke added 3. to review Waiting for reviews 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish 3. to review Waiting for reviews labels Jul 20, 2020
@georgehrke georgehrke force-pushed the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch 2 times, most recently from c3bfb36 to 1f8aae4 Compare July 21, 2020 06:44
@georgehrke
Copy link
Member Author

I'm not sure abou tthis one! Don't we already transpile on the library?
https://github.com/nextcloud/nextcloud-vue/blob/master/babel.config.js

I dug into this issue and turns out it is an issue in v-tooltip. Since version 2.0.3 it's shipping ES6 in the minified builds:
Akryum/floating-vue#466

The server is using v-tooltip 2.0.2, but nc/vue is already on 2.0.3

@georgehrke georgehrke force-pushed the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch from 1f8aae4 to a421723 Compare July 21, 2020 10:36
@danxuliu
Copy link
Member

I have fixed the share link acceptance tests (basically the share menu was covering in some cases the Copy link button; it should not happen with up to date browsers).

However, while checking this I have found a small issue (latest Firefox):

Share-Link-Menu-Out-Of-Bounds

Since nextcloud-libraries/nextcloud-vue@f3b223a the Actions menu is always shown to the top if there is enough room (before this was done only for centered menus, so it did not apply to the share menu due to being aligned to the right). However, as can be seen in this case there is not enough room, and yet the menu is still shown to the top (and a scroll bar is not shown either). I do not know if the problem is in the Vue component itself or in the way that it is used in the sharing tab.

@danxuliu danxuliu removed their assignment Jul 21, 2020
@skjnldsv
Copy link
Member

the problem is in the Vue component itself

this ^

@georgehrke
Copy link
Member Author

the problem is in the Vue component itself

this ^

That seems to be an edge case with certain view heights.
@skjnldsv @rullzer Are you fine with merging this for now?

@skjnldsv
Copy link
Member

@skjnldsv @rullzer Are you fine with merging this for now?

good for me :)

@rullzer
Copy link
Member

rullzer commented Jul 22, 2020

the problem is in the Vue component itself

this ^

That seems to be an edge case with certain view heights.
@skjnldsv @rullzer Are you fine with merging this for now?

Yep lets do this. But be sure to create an issue so we don't lose track

@georgehrke
Copy link
Member Author

Created an issue: nextcloud-libraries/nextcloud-vue#1215

Let me rebase

georgehrke and others added 4 commits July 22, 2020 15:55
Signed-off-by: Georg Ehrke <[email protected]>
The (old) Firefox version used in the acceptance tests does not properly
render the share link menu. As the menu is taller than it should
sometimes it covers the copy link button, so it is not possible to click
it without hiding the share link menu. Moreover, in those cases the
share menu button is also covered by the share menu, so the menu needs
to be closed by pressing the "Esc" key.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@georgehrke georgehrke force-pushed the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch from cbf5a8c to 9303390 Compare July 22, 2020 14:00
@georgehrke georgehrke requested a review from skjnldsv July 22, 2020 14:02
@georgehrke georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 22, 2020
@faily-bot
Copy link

faily-bot bot commented Jul 22, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 30976: failure

mariadb10.1-php7.2

Show full log
There were 8 warnings:

1) Test\DB\DBSchemaTest::testSchema
Using assertContains() with string haystacks is deprecated and will not be supported in PHPUnit 9. Refactor your test to use assertStringContainsString() or assertStringContainsStringIgnoringCase() instead.

2) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "all" (array(0, 1, 2, 3), array(), array())
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

3) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "only-todos" (array(), array('VTODO'), array())
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

4) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "only-events" (array(0, 1, 2, 3), array(), array(array('VEVENT', false, array(), array(null, null), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

5) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "start" (array(1, 2, 3), array(), array(array('VEVENT', false, array(), array(DateTime Object (...), null), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

6) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "end" (array(0), array(), array(array('VEVENT', false, array(), array(null, DateTime Object (...)), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

7) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "future" (array(3), array(), array(array('VEVENT', false, array(), array(DateTime Object (...), null), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

8) OCA\FederatedFileSharing\Tests\FederatedShareProviderTest::testCreate
assertArraySubset() is deprecated and will be removed in PHPUnit 9.

--

There was 1 failure:

1) OCA\Files_Sharing\Tests\SharedMountTest::testPermissionMovedGroupShare with data set #232 ('folder', 29, 13)
Failed asserting that false is true.

/drone/src/apps/files_sharing/tests/SharedMountTest.php:367

--

There was 1 risky test:

1) OCA\TwoFactorBackupCodes\Tests\Db\BackupCodeMapperTest::testInsertArgonEncryptedCodes
This test did not perform any assertions

/drone/src/apps/twofactor_backupcodes/tests/Db/BackupCodeMapperTest.php:115

mysql8.0-php7.2

Show full log
There were 8 warnings:

1) Test\DB\DBSchemaTest::testSchema
Using assertContains() with string haystacks is deprecated and will not be supported in PHPUnit 9. Refactor your test to use assertStringContainsString() or assertStringContainsStringIgnoringCase() instead.

2) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "all" (array(0, 1, 2, 3), array(), array())
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

3) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "only-todos" (array(), array('VTODO'), array())
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

4) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "only-events" (array(0, 1, 2, 3), array(), array(array('VEVENT', false, array(), array(null, null), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

5) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "start" (array(1, 2, 3), array(), array(array('VEVENT', false, array(), array(DateTime Object (...), null), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

6) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "end" (array(0), array(), array(array('VEVENT', false, array(), array(null, DateTime Object (...)), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

7) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testCalendarQuery with data set "future" (array(3), array(), array(array('VEVENT', false, array(), array(DateTime Object (...), null), array())))
The optional $canonicalize parameter of assertEquals() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertEqualsCanonicalizing() instead.

8) OCA\FederatedFileSharing\Tests\FederatedShareProviderTest::testCreate
assertArraySubset() is deprecated and will be removed in PHPUnit 9.

--

There were 2 failures:

1) OCA\Files_Sharing\Tests\SharedMountTest::testPermissionMovedGroupShare with data set #165 ('folder', 13, 7)
Failed asserting that false is true.

/drone/src/apps/files_sharing/tests/SharedMountTest.php:367

2) OCA\Files_Versions\Tests\VersioningTest::testRestoreMovedShare
File content has not changed
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'version 2'
+'version 1'

/drone/src/apps/files_versions/tests/VersioningTest.php:729

--

There was 1 risky test:

1) OCA\TwoFactorBackupCodes\Tests\Db\BackupCodeMapperTest::testInsertArgonEncryptedCodes
This test did not perform any assertions

/drone/src/apps/twofactor_backupcodes/tests/Db/BackupCodeMapperTest.php:115

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 22, 2020
@skjnldsv skjnldsv merged commit 5fea50b into master Jul 22, 2020
@skjnldsv skjnldsv deleted the dependageorg/npm_and_yarn/nextcloud/vue-2.2.1 branch July 22, 2020 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement feature: dependencies high javascript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants