Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port Files navigation to vue #35772

Merged
merged 10 commits into from
Jan 4, 2023
Merged

Port Files navigation to vue #35772

merged 10 commits into from
Jan 4, 2023

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Dec 14, 2022

  • Base Navigation view
  • Navigation API
  • Legacy views API mapper
  • Settings to Modal
  • Component testing

Fix later

  • Breadcrumbs behind navigation collapse button
  • Add copy to clipboard for webdav link
  • Fix favorite live update

Follow-ups

@skjnldsv skjnldsv added this to the Nextcloud 26 milestone Dec 14, 2022
@skjnldsv skjnldsv self-assigned this Dec 14, 2022
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

"@cypress/browserify-preprocessor": "^3.0.2",
"@nextcloud/babel-config": "^1.0.0",
"@nextcloud/cypress": "^1.0.0-beta.1",
"@nextcloud/eslint-config": "^8.0.0",
"@nextcloud/stylelint-config": "^2.1.2",
"@nextcloud/webpack-vue-config": "github:nextcloud/webpack-vue-config#master",
Copy link
Member Author

Choose a reason for hiding this comment

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

no be replaced with newly released webpack-vue-config when released

apps/files/lib/Service/UserConfig.php Fixed Show fixed Hide fixed
apps/files/lib/Service/UserConfig.php Fixed Show fixed Hide fixed
apps/files/lib/Service/UserConfig.php Fixed Show fixed Hide fixed
@@ -88,7 +89,8 @@
$c->get(IPreview::class),
$c->get(IShareManager::class),
$c->get(IConfig::class),
$server->getUserFolder()
$server->getUserFolder(),

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\IServerContainer::getUserFolder has been marked as deprecated
public function showHiddenFiles(bool $show): Response {
$this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'show_hidden', $show ? '1' : '0');
public function showHiddenFiles(bool $value): Response {
$this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'show_hidden', $value ? '1' : '0');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
public function cropImagePreviews(bool $crop): Response {
$this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'crop_image_previews', $crop ? '1' : '0');
public function cropImagePreviews(bool $value): Response {
$this->config->setUserValue($this->userSession->getUser()->getUID(), 'files', 'crop_image_previews', $value ? '1' : '0');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
$response->setStatus(Http::STATUS_FORBIDDEN);
return $response;

$userId = $this->userSession->getUser()->getUID();

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
@@ -205,11 +194,11 @@
// FIXME: Make non static
$storageInfo = $this->getStorageInfo();

$user = $this->userSession->getUser()->getUID();
$userId = $this->userSession->getUser()->getUID();

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
apps/files/lib/Service/UserConfig.php Fixed Show fixed Hide fixed
apps/files/lib/Service/UserConfig.php Fixed Show fixed Hide fixed
@skjnldsv skjnldsv force-pushed the feat/files2vue-navigation branch 4 times, most recently from e087bd5 to 8524f6b Compare December 29, 2022 08:55
* @param string $key a valid config key
* @return string|bool
*/
private function getDefaultConfigValue(string $key): mixed {

Check failure

Code scanning / Psalm

ReservedWord

mixed is a reserved word
@skjnldsv skjnldsv force-pushed the feat/files2vue-navigation branch 2 times, most recently from a9403ba to f8f06c0 Compare December 30, 2022 16:03
@skjnldsv skjnldsv force-pushed the feat/files2vue-navigation branch 8 times, most recently from 4259dad to 10a0c81 Compare January 3, 2023 18:09
@skjnldsv skjnldsv marked this pull request as ready for review January 3, 2023 18:36
@skjnldsv skjnldsv requested review from a team and removed request for a team January 3, 2023 18:36
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Works very well in my testing (apart from the small things that Vincent mentioned) but didnt review the code.

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 no critical issues

the only one that annoys me is the left sidebar behavior but needs handling separately (nextcloud-vue)

apps/files/src/views/Navigation.cy.ts Outdated Show resolved Hide resolved
apps/files/src/views/Settings.vue Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@PVince81
Copy link
Member

PVince81 commented Jan 4, 2023

@skjnldsv please tick the checkboxes and link to tickets for the follow ups to move forward

I did some testing and apart from my above comments I didn't find anything, it works nicely.

Great job! 🎉

@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 Jan 4, 2023
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: John Molakvoæ <[email protected]>
@skjnldsv
Copy link
Member Author

skjnldsv commented Jan 4, 2023

Come on! This time it's the one! Final rebase (™)

@skjnldsv skjnldsv merged commit e235c64 into master Jan 4, 2023
@skjnldsv skjnldsv deleted the feat/files2vue-navigation branch January 4, 2023 16:54
@PVince81
Copy link
Member

PVince81 commented Jan 4, 2023

🎉

@szaimen
Copy link
Contributor

szaimen commented Jan 4, 2023

🎉🎉🎉🎉🎉

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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants