Improve the breadcrumbs component

[nimishavijay]

As discussed with @jancborchardt and @marcoambrosini during the Vue components design review

• The right arrow that separates the folders is super thin and sort of disappears into the bg, so it can be replaced by chevron-right MDI icon
• We can have a better hover feedback, where the background colour is #ededed (--color-background-dark ?) and text is --color-main-text
• Also used home MDI icon here

also cc @skjnldsv for Files

[marcoambrosini]

Looks good. One tiny detail: I would loose the last chevron icon to the right since the new button is sort of in the same context of the current folder

[raimund-schluessler]

I can take care of this. I am just happy you don't want any drastic functional changes (yet), since this component is quite complex 😄.
The home icon is already configurable since the single breadcrumb has a slot for an MD icon. So it is possible to set the correct icon already. However, if the breadcrumb has no icon slot set and you don't specify a rootIcon, it falls back to icon-home which is a standard (non-MD) icon coming from server:
https://github.com/nextcloud/nextcloud-vue/blob/987b0b1d1ded90059f0926534f0cdbc4442444ef/src/components/Breadcrumbs/Breadcrumbs.vue#L119-L122

So I have a couple of questions:

• Do you want me to change the home icon to MDI home by default if no rootIcon is given?
• Do we even need to still allow setting icons by string, or can we completely move to MDI icons only and remove the rootIcon and corresponding icon string prop? This would make it a breaking change, but I think that is not critical, since the component is not yet used in any official Nextcloud app. Plus we would then really enforce MD icons.
• You don't mention the rounded corners of the hover feedback, but show it in the video. I suppose you want them?
• Is the arrow down of the last breadcrumb desired? If so, should we use
  menu-down:
  
  or chevron-down:
  

[skjnldsv]

• Do we even need to still allow setting icons by string, or can we completely move to MDI icons only and remove the rootIcon and corresponding icon string prop? This would make it a breaking change, but I think that is not critical, since the component is not yet used in any official Nextcloud app. Plus we would then really enforce MD icons.

Will be in 24. So I'm fine using a slot if needed. But we need to make sure we have this soon so we don't break it mid-schedule :)

[raimund-schluessler]

Will be in 24. So I'm fine using a slot if needed. But we need to make sure we have this soon so we don't break it mid-schedule :)

If you only use the icon slot and not the icon prop (as anyway required for MD icons), there won't be any problem if we remove the icon prop. Anyway, I will have a look soon.

[raimund-schluessler]

I just saw we also need to show an MD icon by default for the hidden breadcrumbs in the dropdown at
https://github.com/nextcloud/nextcloud-vue/blob/987b0b1d1ded90059f0926534f0cdbc4442444ef/src/components/Breadcrumbs/Breadcrumbs.vue#L500
We currently just show icon-folder from server.

I will also check if it might also be possible to kind of forward the breadcrumb icon slot to the ActionButton icon slot, in case the breadcrumb is hidden.

[raimund-schluessler]

The first step is in #2411.

[raimund-schluessler]

Todo:

• Use ChevronRight as separator Adjust breadcrumb appearance #2411
• Show rounded background on hover Adjust breadcrumb appearance #2411
• Fix dragging onto hidden crumbs Fix dragging onto hidden crumbs #2412
• Use MD icon Folder in hidden crumbs dropdown Use MD icon for hidden crumbs action button icons #2413
• Implement slot for customizing the Actions trigger to show MenuDown for the last breadcrumb Allow custom action menu icon for breadcrumb #2414
• Use constant for crumb class name vue-crumb in Breadcrumbs Add variable for crumb class name #2450
• Fall back to MD icon home as root icon if not specified differently
• Use provided crumb icons in hidden crumbs dropdown

[nimishavijay]

Do you want me to change the home icon to MDI home by default if no rootIcon is given?

Yes, since we're moving towards standardising MDI this would be a good idea

You don't mention the rounded corners of the hover feedback, but show it in the video. I suppose you want them?

Haha, yes! Looks great in #2411!

Is the arrow down of the last breadcrumb desired?

Yeah, so ideally we were thinking of having an action menu there next to the current folder instead of a share icon, so on clicking it the action menu for the folder would appear.
I like the idea of using chevron-down but I tried it out and it looks a little bit confusing as it's too similar to chevron-right

I'd say we can go with menu-down here

[raimund-schluessler]

Yeah, so ideally we were thinking of having an action menu there next to the current folder instead of a share icon, so on clicking it the action menu for the folder would appear.

This can be done already. If you supply e.g. multiple ActionButton they are rendered in an Actions dropdown with a three-dots menu. I guess we just need to create a slot allowing to customize the trigger icon so that we can throw in MenuDown instead of the three-dots for example.

[raimund-schluessler]

Reopening, since we are not done yet 🙂

[quentinguidee]

Happy to see improvement on the breadcrumbs! The only issue I see here is about the paddings. Currently, because the button have a big height, the circles visually (in mind) cut the text and one has the impression of being tight. Currently :

Would be better in my opinion to have something like this :

Principle also found in material design guidelines on this big round buttons :

Or on smaller buttons (also material design, or with Nextcloud) :

[raimund-schluessler]

Would be better in my opinion to have something like this :

I agree that it looks better for the single (hovered) breadcrumb. The crumbs are 44 px high, which means we need 22 px left/right padding to achieve what you propose (border radius ends at the position of the text). The problem is that if we do that and you don't hover over a breadcrumb, there is to much white space. And we can show less breadcrumbs, obviously.

See:

The background and problem are only visible on hover. So I would say it is better to have a nice looking and better functional breadcrumb bar when you don't hover any crumb (which is the default state) than account for an issue that only happens rarely (on hover).

[quentinguidee]

Yes, I realized the problem while doing it in devtools. But I think the problem is viewed from the wrong angle here. If the necessary horizontal margins are too large, then it's because the button is too high. Even if the "bug" is only visible when you pass over it, it remains an unpleasant visual bug for the end user. I don't think prioritizing maximum size goes over a nice interface (just as a nice interface doesn't go over functionality, the two should be judged equally).

[quentinguidee]

And for info Google Drive uses a 32px height breadcrumbs, so I don't think a button that high is necessary in this context.

[raimund-schluessler]

If the necessary horizontal margins are too large, then it's because the button is too high.

I also thought about that and it would really solve or at least reduce the problem. But I don't know if that is desired here, since the general rule all over Nextcloud is that any clickable element should be at least 44 px times 44 px to make it accessible by touch as well. Reducing the height to e.g. 32 px would violate this principle.

Edit: That's how it would look with 32 px for example. Please note how the share icon is not aligned since it uses the standard 44 px height of course:

[quentinguidee]

Ok, thanks! Didn't know that it should be accessible by touch by default. But then what about this button from NextCloud (32px too) ?

Seems totally usable by touch, or maybe an intermediate size would be ideal, like 38px ?

I think that even in mobile apps the buttons are not that high :

Source: material design v3

[raimund-schluessler]

Ok, thanks! Didn't know that it should be accessible by touch by default. But then what about this button from NextCloud (32px too) ?

Yeah, I am afraid Nextcloud is not totally adhering to these rules everywhere yet. That's also why we now e.g. have a button component in nextcloud/vue so it looks the same everywhere (once it's used everywhere 🙈).

But this discussion is more for @jancborchardt and @nimishavijay, since they do the Nextcloud design 😉

[quentinguidee]

Ok, thanks for your comments! 👍

For illustration purpose, here's what a 38px button would look like

Or a 40px height (with the minimal horizontal padding possible with this height) :

(preference for the first one, better visually I think)

[raimund-schluessler]

Another option would be to use the border-radius: 16 px; from the ListItem.
https://github.com/nextcloud/nextcloud-vue/blob/bf562d8c7377bd6b32b50f16aad34660f7c3da44/src/components/ListItem/ListItem.vue#L429-L435
Together with 16 px left/right padding it looks like this (still a bit much white-space, but much less than 22 px would create):

I think this could even make sense, since it creates a relation to the ListItems which will be used in the new Files app, as far as I know. Both elements are for navigating and will bring you to another view, rather than merely triggering an Action on the same site as a button would.

But I think this is enough input for @nextcloud/designers now. Your call. 🙂

[marcoambrosini]

@raimund-schluessler I think that works!