enh(settings): Migrate admin settings for sharing to vue by susnux · Pull Request #41581 · nextcloud/server

susnux · 2023-11-17T16:02:56Z

Resolves: [BITV] Replace all places with old NcMultiselect components with NcSelect components #40588

Summary

Migrate sharing settings to get rid of select2 group select. Needed for accessibility.
Also fixes the not working "exclude groups from sharing" select.

Screenshots

before	after

Screen cast

vokoscreenNG-2023-11-17_17-29-26.mp4

Checklist

Code is properly formatted
Sign-off message is added to all commits
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

JuliaKirschenheuter

🥳 very nice!

susnux · 2023-11-17T18:49:41Z

/compile amend/

ShGKme · 2023-11-17T22:13:54Z

There is an issue with textarea:

I see a different state sometimes, not sure if it is an issue.

master	pr

susnux · 2023-11-17T22:21:49Z

There is an issue with textarea:

Will be fixed with the nextcloud-vue update

I see a different state sometimes, not sure if it is an issue.

Thats because there is no such setting as disclaimer "disabled", before it just checks whether the disclaimer is falsy or truthy.
To distinguish I used following logic: If it is undefined then it is disabled, otherwise enabled.
It will be undefined if the setting is deleted on server, if you enable an empty string will be set on server.

ShGKme · 2023-11-20T09:37:09Z

Dron error is unrelated

This is required to get the fixes for a11y from `@nextcloud/vue`. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

…on` which are expected by `@nextcloud/vue` Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

come-nc · 2023-11-21T08:19:26Z

Drone error very related, this broke tests for master:

452	There were 4 errors:
453	
454	1) OCA\Settings\Tests\Settings\Admin\SharingTest::testGetFormWithoutExcludedGroups
455	ArgumentCountError: Too few arguments to function OCA\Settings\Settings\Admin\Sharing::__construct(), 4 passed in /drone/src/apps/settings/tests/Settings/Admin/SharingTest.php on line 68 and exactly 7 expected
456	
457	/drone/src/apps/settings/lib/Settings/Admin/Sharing.php:48
458	/drone/src/apps/settings/tests/Settings/Admin/SharingTest.php:68
459	
460	2) OCA\Settings\Tests\Settings\Admin\SharingTest::testGetFormWithExcludedGroups
461	ArgumentCountError: Too few arguments to function OCA\Settings\Settings\Admin\Sharing::__construct(), 4 passed in /drone/src/apps/settings/tests/Settings/Admin/SharingTest.php on line 68 and exactly 7 expected
462	
463	/drone/src/apps/settings/lib/Settings/Admin/Sharing.php:48
464	/drone/src/apps/settings/tests/Settings/Admin/SharingTest.php:68
465	
466	3) OCA\Settings\Tests\Settings\Admin\SharingTest::testGetSection
467	ArgumentCountError: Too few arguments to function OCA\Settings\Settings\Admin\Sharing::__construct(), 4 passed in /drone/src/apps/settings/tests/Settings/Admin/SharingTest.php on line 68 and exactly 7 expected
468	
469	/drone/src/apps/settings/lib/Settings/Admin/Sharing.php:48
470	/drone/src/apps/settings/tests/Settings/Admin/SharingTest.php:68
471	
472	4) OCA\Settings\Tests\Settings\Admin\SharingTest::testGetPriority
473	ArgumentCountError: Too few arguments to function OCA\Settings\Settings\Admin\Sharing::__construct(), 4 passed in /drone/src/apps/settings/tests/Settings/Admin/SharingTest.php on line 68 and exactly 7 expected
474	
475	/drone/src/apps/settings/lib/Settings/Admin/Sharing.php:48
476	/drone/src/apps/settings/tests/Settings/Admin/SharingTest.php:68

nickvergessen · 2023-11-21T08:28:28Z

Please update the documentation screenshots:
https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/file_sharing_configuration.html

artonge · 2023-11-21T13:13:06Z

Fix attempt here: #41637

susnux · 2023-11-22T11:09:07Z

Please update the documentation screenshots

Done nextcloud/documentation#11300

susnux added bug 3. to review Waiting for reviews feature: settings accessibility labels Nov 17, 2023

susnux added this to the Nextcloud 28 milestone Nov 17, 2023

susnux requested review from a team, JuliaKirschenheuter, Pytal, emoral435, nfebe and sorbaugh and removed request for a team November 17, 2023 16:02

susnux force-pushed the fix/migrate-admin-settings-sharing-vue branch 3 times, most recently from f8e8064 to 439e03c Compare November 17, 2023 16:29

JuliaKirschenheuter approved these changes Nov 17, 2023

View reviewed changes

JuliaKirschenheuter assigned susnux Nov 17, 2023

JuliaKirschenheuter requested a review from ShGKme November 17, 2023 16:30

Pytal approved these changes Nov 17, 2023

View reviewed changes

susnux force-pushed the fix/migrate-admin-settings-sharing-vue branch from 439e03c to e5800f6 Compare November 17, 2023 18:23

ShGKme approved these changes Nov 17, 2023

View reviewed changes

susnux force-pushed the fix/migrate-admin-settings-sharing-vue branch from e5800f6 to 92c351b Compare November 17, 2023 23:09

susnux enabled auto-merge November 17, 2023 23:09

blizzz mentioned this pull request Nov 20, 2023

28.0.0 RC1 #41621

Merged

5 tasks

susnux added 2 commits November 20, 2023 17:07

enh(settings): Migrate admin settings for sharing to vue

22163c6

This is required to get the fixes for a11y from `@nextcloud/vue`. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

fix(config): Adjust webpack config to provide appName and `appVersi…

8fc0395

…on` which are expected by `@nextcloud/vue` Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

chore: Compile assets

543e2db

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

susnux force-pushed the fix/migrate-admin-settings-sharing-vue branch from 92c351b to 543e2db Compare November 20, 2023 16:10

susnux disabled auto-merge November 20, 2023 16:31

susnux merged commit 02807a3 into master Nov 20, 2023

susnux deleted the fix/migrate-admin-settings-sharing-vue branch November 20, 2023 16:32

nickvergessen added the pending documentation This pull request needs an associated documentation update label Nov 21, 2023

artonge mentioned this pull request Nov 21, 2023

Fix sharing settings unit tests #41637

Merged

susnux mentioned this pull request Nov 22, 2023

fix(admin): Update files sharing documentation for new admin settings nextcloud/documentation#11300

Merged

skjnldsv removed the pending documentation This pull request needs an associated documentation update label Apr 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

enh(settings): Migrate admin settings for sharing to vue#41581

enh(settings): Migrate admin settings for sharing to vue#41581
susnux merged 3 commits intomasterfrom
fix/migrate-admin-settings-sharing-vue

susnux commented Nov 17, 2023 •

edited

Loading

Uh oh!

JuliaKirschenheuter left a comment

Uh oh!

susnux commented Nov 17, 2023

Uh oh!

ShGKme commented Nov 17, 2023

Uh oh!

susnux commented Nov 17, 2023

Uh oh!

ShGKme commented Nov 20, 2023

Uh oh!

come-nc commented Nov 21, 2023

Uh oh!

nickvergessen commented Nov 21, 2023

Uh oh!

artonge commented Nov 21, 2023

Uh oh!

susnux commented Nov 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

susnux commented Nov 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screenshots

Screen cast

Checklist

Uh oh!

JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

Uh oh!

susnux commented Nov 17, 2023

Uh oh!

ShGKme commented Nov 17, 2023

Uh oh!

susnux commented Nov 17, 2023

Uh oh!

ShGKme commented Nov 20, 2023

Uh oh!

come-nc commented Nov 21, 2023

Uh oh!

nickvergessen commented Nov 21, 2023

Uh oh!

artonge commented Nov 21, 2023

Uh oh!

susnux commented Nov 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

susnux commented Nov 17, 2023 •

edited

Loading