Create an app configuration

[MarcelRobitaille]

Fixes #1067

Move the AppNavigationSettings from the sidebar to a AppSettingsDialog modal that covers the screen.

This is what it should look like:

This makes it easy for us to add more configuration options for things like notifications and alarms without cluttering the sidebar.

[github-actions]

Unit Test Results

     27 files       27 suites   7m 4s ⏱️
   476 tests    476 ✔️ 0 💤 0 ❌
4 284 runs  4 283 ✔️ 1 💤 0 ❌

Results for commit 5e5cb6f.

♻️ This comment has been updated with latest results.

[MarcelRobitaille]

@christianlupus That's weird, running npm run styleling locally doesn't warn about anything for me. I ran prettier, eslint, and styleling before pushing. BTW, I'm getting a deprecation warning with eslint now.

[christianlupus]

Are you on the latest styleling config? There was a PR recently #1254.

[MarcelRobitaille]

@christianlupus I guess I branched off before that. Also, the eslint warning was saying that ~/.eslintrc is deprecated, so it has nothing to do with this project. Sorry.

[christianlupus]

The PR has some problems yet. The component NcButton does not yet exist. It is part of the @nextcloud/vue v6 package. We are stuck with v5. Like this, the button is non-functional.

When changing the button, I get an error in the JS console:

Additionally, I cannot change the folder of the cookbook. I cannot navigate in the modal dialog to select a folder.

Can you confirm these issues?

[MarcelRobitaille]

@christianlupus I called it NcButton and not Button because I got an error something like Button is a reserved word in HTML. There shouldn't be a problem with import NcButton from '@nextcloud/vue/dist/Components/Button, is there? It's fewer things to change when we finally migrate too.

I will have a look at resolving the other problems.

Thanks for reviewing 😄.

[MarcelRobitaille]

Here is the error when renaming NcButton to Button:

/home/marcel/code/cookbook/src/components/SettingsDialog.vue
  101:9  error  Name "Button" is reserved in HTML  vue/no-reserved-component-names

I think that is the cause of the errors in the screenshot you posted.

[christianlupus]

@christianlupus I called it NcButton and not Button because I got an error something like Button is a reserved word in HTML. There shouldn't be a problem with import NcButton from '@nextcloud/vue/dist/Components/Button, is there? It's fewer things to change when we finally migrate too.

Ahh, I am sorry, I missed that line. I assumed you were using import {NcButton} from .... You are perfectly fine with that. Sorry for bothering you.

Thanks for reviewing smile.
You are welcome.

Please tell me when you finished with the entire PR. Thanks!

[MarcelRobitaille]

No worries.

I fixed the problem with the reindex button.

I am not sure what is going on with the file picker. Maybe it doesn't like having 2 overlays open at a time. I added this code in the developer console, and once the filepicker dialog is open, no events are fired at all:

document.addEventListener('click', e => console.log(e))

I'm not really sure where this global OC is coming from. I can't find any documentation for it. I don't really like this kind of magic API, I prefer to import what I'm using explicitly. It's different from this, right?

I was not able to reproduce the error in your screenshot. Did you use the code exactly as it is here or did you replace NcButton with Button locally?

[christianlupus]

I'm not really sure where this global OC is coming from. I can't find any documentation for it. I don't really like this kind of magic API, I prefer to import what I'm using explicitly. It's different from this, right?

I do not know for sure. It comes from here. I have read of it in the past but I do no longer find it.

I was not able to reproduce the error in your screenshot. Did you use the code exactly as it is here or did you replace NcButton with Button locally?

Yes, I changed that locally. I have to retest it.

[MarcelRobitaille]

I found the function: https://github.com/nextcloud/server/blob/master/core/src/OC/dialogs.js#L229-L572

It seems like a bug, but I'm not sure whether to report it to @nextcloud/vue or nextcloud/server.

[MarcelRobitaille]

After hours of debugging, I found that this bug is not present in @nextcloud/vue <= 5.3.1. We could downgrade to this version until we move to 7.0.0, or we could try to upstream a patch and have it backported to 5.4.1.

[MarcelRobitaille]

My guess is that the cause is nextcloud-libraries/nextcloud-vue#2654. It seems like it was already fixed in later versions. Should we request a backport?

[christianlupus]

I think a backport might be the cleanest solution.

Let's see, if @nextcloud/vuejs do know something about this issue. Can anyone of you confirm that there might be a problem and there is in fact a chance this can be fixed with the 5.x branch?

[github-actions]

Test Results

     21 files     952 suites   5m 19s ⏱️
   495 tests    495 ✔️ 0 💤 0 ❌
3 465 runs  3 464 ✔️ 1 💤 0 ❌

Results for commit b0dd3c6.

♻️ This comment has been updated with latest results.

[christianlupus]

ATM there are some commits that need fixing. I will wait and set this PR to be a draft. You can mark as ready for review once, you are done, @MarcelRobitaille.

[MarcelRobitaille]

I rebased the NC25 changes. Now we're getting hung up on this open issue: nextcloud-libraries/nextcloud-vue#3386

[christianlupus]

👍 for the rebase, 👎 for the upstream bug.

[MarcelRobitaille]

The upstream issue was closed, but the fixes still haven't made it into a stable version.

[MarcelRobitaille]

nextcloud/vue 7.10.0 was recently released, which fixes the "too much recursion" issue. I've just tested it and now everything seems to be working. The settings modal works and I can interact with the "Recipe folder" modal.

[christianlupus]

Seems good to me now. LGTM