Move subscribe button to search area of Navigation

[lgabardos]

• Resolves: Subscribe button container has a scrollbar #2931

Summary

The subscribe button is inside a container which is expecting to have more entries
The NcAppNavigation is having a dedicated place where an action button should belong, it's in the "search" slot (according to Nextcloud Contact app )

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Changelog entry added for all important changes.

[wofferl]

Thanks for your PR, but as I said in the discussions thread the #search slot has its own purpose and unlike the contact app, news has no in-app search.

search For in-app search you can pass a NcAppNavigationSearch component as the slot content.

Looking where NcAppNavigationNew is used, e.g. global accounts setting in nextcloud, in the notes app, etc where it behave the same, let me think that this is a bug in the component, which should reported upstream.

[lgabardos]

Thanks for your PR, but as I said in the discussions thread the #search slot has its own purpose and unlike the contact app, news has no in-app search.

search For in-app search you can pass a NcAppNavigationSearch component as the slot content.

Looking where NcAppNavigationNew is used, e.g. global accounts setting in nextcloud, in the notes app, etc where it behave the same, let me think that this is a bug in the component, which should reported upstream.

Let’s wait for your feedback on that
As I said, I’m not a big fan of the « css way » to fix the issue.
Though, it might be a bug in the global component.

[wofferl]

Let’s wait for your feedback on that As I said, I’m not a big fan of the « css way » to fix the issue. Though, it might be a bug in the global component.

Reading Nextcloud Vue Components it seems to be no bug, but you need to mock the content like described in the examples.

e.g.:

	<style scoped>
		/* Mock NcContent */
		.styleguide-nc-content {
			overflow: hidden;
		}
	</style>

You can do this if you want and if so you can also move the "new folder" button above, this should not belong to the list.

[lgabardos]

Let’s wait for your feedback on that As I said, I’m not a big fan of the « css way » to fix the issue. Though, it might be a bug in the global component.

Reading Nextcloud Vue Components it seems to be no bug, but you need to mock the content like described in the examples.

e.g.:

	<style scoped>
		/* Mock NcContent */
		.styleguide-nc-content {
			overflow: hidden;
		}
	</style>

You can do this if you want and if so you can also move the "new folder" button above, this should not belong to the list.

I'm adding more intel
nextcloud-libraries/nextcloud-vue#6079
nextcloud/polls#3704

I think it's an issue of the "default" template on the NcAppNavigation
I don't understand why the css class ".app-navigation__body" has a "overflow-y: scroll". It forces the content to have a scroll and then it's cropped.

For this PR, I'm thinking to do the same as "polls", use a "hack" css to display properly the ".app-navigation__body".
So, I wanted to put the css change in "~/css/navigation.scss" but it seems that all scss are not used in the app? Am I wrong?

[wofferl]

For this PR, I'm thinking to do the same as "polls", use a "hack" css to display properly the ".app-navigation__body". So, I wanted to put the css change in "~/css/navigation.scss" but it seems that all scss are not used in the app? Am I wrong?

You are right, this seems to be a leftover from news 24, but I'm also not yet through what can be removed.
For this I think the right place is a <style scoped> in Sidebar.vue as shown in the Nextcloud Component documentation.

[lgabardos]

For this PR, I'm thinking to do the same as "polls", use a "hack" css to display properly the ".app-navigation__body". So, I wanted to put the css change in "~/css/navigation.scss" but it seems that all scss are not used in the app? Am I wrong?

You are right, this seems to be a leftover from news 24, but I'm also not yet through what can be removed. For this I think the right place is a <style scoped> in Sidebar.vue as shown in the Nextcloud Component documentation.

Unfortunately, the style scoped cannot be used here because the [data-xxx] applied to the scoped css cannot target the ".app-navigation__body" class
it needs to be in a global place...
I don't know the Nextcloud guidelines for such changes...

• a <style> tag in the Sidebar.vue
• add css class in App.vue (which already has a <style> tag)
• Add a "hack.scss" and link it in App.vue

I got a preference for the last one

[wofferl]

For this PR, I'm thinking to do the same as "polls", use a "hack" css to display properly the ".app-navigation__body". So, I wanted to put the css change in "~/css/navigation.scss" but it seems that all scss are not used in the app? Am I wrong?

You are right, this seems to be a leftover from news 24, but I'm also not yet through what can be removed. For this I think the right place is a <style scoped> in Sidebar.vue as shown in the Nextcloud Component documentation.

Unfortunately, the style scoped cannot be used here because the [data-xxx] applied to the scoped css cannot target the ".app-navigation__body" class it needs to be in a global place... I don't know the Nextcloud guidelines for such changes...

* a <style> tag in the Sidebar.vue

* add css class in App.vue (which already has a <style> tag)

* Add a "hack.scss" and link it in App.vue

I got a preference for the last one

This should work in Sidebar.vue

<style scoped>
/*
 * workaround remove extra scroll bar in navigation body
 */
:deep(.app-navigation__body) {
  overflow: unset;
}
</style>

[wofferl]

Thanks. Please fix the linter errors showing by npm run lint and add a changelog entry.