Split AppNavigationItem into small components

[skjnldsv]

Same as https://github.com/nextcloud/nextcloud-vue/pull/358 and after #25 https://github.com/nextcloud/nextcloud-vue/pull/195 Refs: https://docs.nextcloud.com/server/16/developer_manual/design/navigation.html#app-navigation-menu

• [x] Entry with counter
• [x] Use Actions components
• [x] Collapsible component
• [x] Undo (with transition: slide)
• [x] Edit (with transition: fade)
• [x] Pinned item
• [x] Nested AppNavigationItem

<AppNavigationItem counter="test">
    This is the main entry
    <AppNavigationItem>Sub-element 1</AppNavigationItem>
    <AppNavigationItem>Sub-element 2</AppNavigationItem>
    <template #actions>
        <ActionButton @click="doSomething" />
    </template>
</AppNavigationItem>

[marcoambrosini]

Hello everyone, Marco here! I'll be helping out for the next few months :)

I see 2 possible approaches in terms of organising the code. These two can also be mixed together, but I'm gonna describe here the two edge cases just to be clear:

#1 We could use all the and <AppNavigationWhatever /> and <Actions /> components directly in App.vue by nesting them into the <AppNavigationItem /> component. In this case we'd treat <AppNavigationItem /> almost like a wrapper:

<AppNavigationItem>
    This is the main entry
    <Avatar #avatar user="admin" display-name="Administrator" /> 
    <AppNavigationCounter #counter>123</AppNavigationCounter> 
    <template #actions>
        <ActionButton />
        <ActionButton />
        <ActionButton />
    <template>
</AppNavigationItem>

#2 We could make the <AppNavigationItem /> component a way smarter (kinda like it is now) and pass down everything as props, do the checks within the component and display the subcomponents (actions, avatar, counters etc) based on those checks. Something more like:

 <AppNavigationItem counter="123" avatar="http://xxxx.xxx"  actions="{ [ ... , ... , ... ] }" >

In this case the only components nested into <AppNavigationItem /> could be anoter <AppNavigationItem /> or`.

My personal preference is #1, I think that even if it might add more clutter at App.vue level, it would make things more readable at a glance. What do you think @skjnldsv @juliushaertl @georgehrke??

[skjnldsv]

Other examples:

<AppNavigationItem class="icon-xxx">This is the main entry</AppNavigationItem>

<!-- https://docs.nextcloud.com/server/16/developer_manual/design/navigation.html#entry-bullet -->
<AppNavigationItem bullet="0082c9">This is the main entry with a bullet</AppNavigationItem>

<AppNavigationItem class="icon-xxx">
    This is the main entry
    <AppNavigationCounter #counter :highlighted="true">123</AppNavigationCounter>
</AppNavigationItem>

<AppNavigationItem class="icon-xxx">
    This is the main entry
    <AppNavigationCounter #counter :highlighted="true">123</AppNavigationCounter>
    <AppNavigationItem>This is a sub-entry</AppNavigationItem>
    <AppNavigationItem>This is a sub-entry</AppNavigationItem>
    <AppNavigationItem>This is a sub-entry</AppNavigationItem>
</AppNavigationItem>

<AppNavigationItem class="icon-xxx" :collapsible="true">
    This is the main entry
    <AppNavigationItem>This is a sub-entry</AppNavigationItem>
    <AppNavigationItem>This is a sub-entry</AppNavigationItem>
    <AppNavigationItem>This is a sub-entry</AppNavigationItem>
    <template #actions>
        <ActionButton />
        <ActionButton />
        <ActionButton />
    <template>
</AppNavigationItem>

We still need to figure out the undo and edit mode. @nextcloud/vue any ideas? Should we create a AppNavigationEdit element? Or a prop toggle :undo="true"? Or just let the dev strike the text and put a icon-history undo action? Then I guess we could only have to think about the edit mode? https://docs.nextcloud.com/server/16/developer_manual/design/navigation.html#edit-entry

[korelstar]

When looking at the discussion in #447, I suggest to use a separate template slot for children items. Otherwise it will be hard to distinct between children and main elements.

Regarding undo, I prefer a prop toggle.

[skjnldsv]

Regarding undo, I prefer a prop toggle.

Any specific reasons? :)

Otherwise it will be hard to distinct between children and main elements.

What elements do yuo have in mind?

[marcoambrosini]

I agree with @korelstar about the undo, I think that anything that would allow to change AppNavigationItem, It's behaviour (i.e. always open children), and it's title (i.e. edit) should be a prop. As per the nested components I don't think that there's the need to use templates, with proper naming and indentation it should look pretty clear right?

[korelstar]

Regarding undo, I prefer a prop toggle.

Any specific reasons? :)

It's because I think that the component should provide a semantic for undo. The design functionality should be capsulated in the component. This would make design changes (e.g. animations) easier and streamlined through different apps.

---

Otherwise it will be hard to distinct between children and main elements.

What elements do yuo have in mind?

Nothing concrete, but in the context of #420, #445 and #447 I learned, that it is better to make things explicit. You can't check for a child component by name, because someone could use it's own component which is based on AppNavigationItem (I'm doing this in notes). And you should not assume that all child components are of the expected type, because someone could add a child element for another purpose (e.g. colorize a single word).

[skjnldsv]

You can't check for a child component by name, because someone could use it's own component which is based on AppNavigationItem

Well, if you have to use your own custom component, it means our current one is not good enough and needs to be perfected :) This is where we have the balance between a standardized components set and devs doing whatever they want!

[korelstar]

Well, if you have to use your own custom component, it means our current one is not good enough and needs to be perfected :)

This doesn't apply in my case. While AppNavigationItem is a generic component, I want to use a subject-specific component (NavigationNoteItem, see link above) which uses the generic component but implements semantic for the specific domain. This can't be integrated in the generic component.

In other languages, I would use inheritance for that (NavigationNoteItem would be a subclass of AppNavigationItem), but this is not possible with Vue components.

[skjnldsv]

Ah yes! We should indeed be able to use renderless components !

[marcoambrosini]

@skjnldsv what should I do with the CSS? I was starting to move it from the server to the Vue components, but in the classes there are many references to mixins and variables that live on the server. I'm not able to compile the code because the compiler looks for those references in the nextcloud-vue folder and doesn't find them. In theory, once you're developing the apps using the npm package, these componends should have access to those mixins and variables right?