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

Move crumb Actions into hover background area #2416

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

raimund-schluessler
Copy link
Contributor

@raimund-schluessler raimund-schluessler commented Dec 22, 2021

This adjusts the appearance of the breadcrumb bar to have the Actions included in the hover background area and makes the last breadcrumb bold. The Actions menu is now centered below the crumb and clicking anywhere on the crumb opens it. For the first crumb, we removed the hover effect.

Looks like this (please note the misaligned arrow-down icon is a bug in the Actions component and will be fixed with #2435):
crumb

@quentinguidee
Copy link
Contributor

We could change that so the menu is also opened when clicking on the text

Personally, I would also go in this direction (and this fixes the fact that the text is glued to the round button). By the way this solution is also the behavior adopted by google drive, (but they put an arrow instead of the dots to make it really clear that the last is a dropdown menu and others will just navigate).

since clicking on a crumb normally navigates you to the respective folder/location

So this solution fixes that, because they will be like two separate buttons styles.

Plus, clicking on the last button should only be used to download/share/... while others only used to navigate.

@raimund-schluessler
Copy link
Contributor Author

We could change that so the menu is also opened when clicking on the text

(but they put an arrow instead of the dots to make it really clear that the last is a dropdown menu and others will just navigate).

We will also have an arrow down, but that requires #2414.

since clicking on a crumb normally navigates you to the respective folder/location

So this solution fixes that, because they will be like two separate buttons styles.

What do you mean with two separate button styles, the difference between a crumb with and without MenuDown icon?

@quentinguidee
Copy link
Contributor

quentinguidee commented Dec 22, 2021

What do you mean with two separate button styles, the difference between a crumb with and without MenuDown icon?

Yeah, for me when I pass my mouse over a button and I see that the arrow is took with it, it's like a "dropdown menu" component, whereas the version without the arrow looks like a simple "button" component. Clicking on a button that has an arrow in the hover zone the user should know immediately that he will not navigate

I would also add that when a user clicks on the last breadcrumb, it is never to browse, am I wrong? If that's the case, the user clicking on the last breadcrumb will always want the dropdown behaviour, and when the user clicks on others he only want the navigation behaviour

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Dec 22, 2021

Fine with me. If we could merge #2412 and #2414, I will rebase and adjust this PR here tomorrow so that clicking on a crumb with an Action opens the dropdown menu.

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Dec 23, 2021

Rebased the branch and updated the screenshots to show MenuDown as Action menu trigger icon.

Input from @nextcloud/designers would be very welcome.

Copy link
Contributor

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

I'd merge the hover feedbacks, like they do in google drive

Screen.Recording.2021-12-28.at.12.25.30.mov

@nimishavijay
Copy link

nimishavijay commented Jan 4, 2022

Looks great with the MenuDown icon now! Agreed with the others, it looks less messy if you get rid of the hover feedback for the menu icon. We can keep it similar to Google where clicking on the current folder text or the icon opens the menu. The icon is there to indicate that there are more actions available.

@raimund-schluessler
Copy link
Contributor Author

Agreed with Marco, I think it looks less messy if you get rid of the hover feedback for the 3 dot icon. We can keep it similar to Google.

Ok. I will take care of this.
I just have one question remaining. How should it look like if it's not a dropdown, but a single Action showing directly. Should we keep the separate hover feedback and give it left margin like so:

Untitled

Or handle it differently?

@jancborchardt
Copy link
Contributor

Yup, agree with everyone’s feedback. :) As also said in my original comment:

The whole "Folder 5" entry should be clickable in this case, basically be a button with an icon on the right.

Also, the dropdown menu can be centered below the entire button, not below the triangle icon, cause that looks like it’s aligned to the right. Makes it simpler to just move the mouse / finger down. :)

@raimund-schluessler regarding your question:

I just have one question remaining. How should it look like if it's not a dropdown, but a single Action showing directly. Should we keep the separate hover feedback and give it left margin like so

No, I’d say in that case it can also be the same as when it’s a dropdown, because for a single action the whole folder entry should be clickable too.

@raimund-schluessler
Copy link
Contributor Author

Yup, agree with everyone’s feedback. :) As also said in my original comment:

The whole "Folder 5" entry should be clickable in this case, basically be a button with an icon on the right.

Also, the dropdown menu can be centered below the entire button, not below the triangle icon, cause that looks like it’s aligned to the right. Makes it simpler to just move the mouse / finger down. :)

Ok, I will have a look how to do that. We will likely need to adjust the Actions component to allow showing the icon on the right instead of on the left.

I just have one question remaining. How should it look like if it's not a dropdown, but a single Action showing directly. Should we keep the separate hover feedback and give it left margin like so

No, I’d say in that case it can also be the same as when it’s a dropdown, because for a single action the whole folder entry should be clickable too.

Good, that makes it easier to implement.

@raimund-schluessler
Copy link
Contributor Author

Requires #2436.

@raimund-schluessler raimund-schluessler added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 4, 2022
@raimund-schluessler raimund-schluessler self-assigned this Jan 4, 2022
@raimund-schluessler
Copy link
Contributor Author

@jancborchardt @nimishavijay One last question, hopefully 🙈 How should the menu look in case of one crumb only (e.g. when being in the root folder) where we show an icon only. Should it look like this?

Screenshot 2022-01-05 at 11-09-13 Nextcloud Vue Style Guide

  • Icon for the root folder and an icon (arrow down) for indicating having Actions
  • Hover feedback for both
  • Actions menu centered below

If so, we will need to adjust the Actions component to allow showing an icon instead of the menu-title plus having an icon for indicating the Action menu toggle. Would this be ok?

@jancborchardt
Copy link
Contributor

@raimund-schluessler good question – to be honest I don’t think the root folder needs these actions.

  • Sharing the root folder is not possible currently as far as I understand?
  • Download can easily be done via select all → download

So I’d say when you are in the root, just show the home icon (doesn’t get any hover/focus feedback cause it’s not clickable), then the chevron divider, then the "+ New" action button.

@raimund-schluessler
Copy link
Contributor Author

@raimund-schluessler good question – to be honest I don’t think the root folder needs these actions.

  • Sharing the root folder is not possible currently as far as I understand?

  • Download can easily be done via select all → download

That's true, there currently is no Action for the root crumb in the Files app. I just thought to have it consistent and possibly allow Actions for every breadcrumb level, as other apps might have a use case for it (I could think of one for an app of mine, where clicking the root crumb would open the details panel). But we can also do this in a follow-up in case we need it.

So I’d say when you are in the root, just show the home icon (doesn’t get any hover/focus feedback cause it’s not clickable), then the chevron divider, then the "+ New" action button.

@jancborchardt You want the chevron divider in this case? Would be inconsistent with the other crumb levels where you decided not to show the chevron for the last crumb, no?
The "+ new" button is not part of the breadcrumb bar, so nothing to do here.

Anyway, I will implement what we discussed now and we can have a look again, once a realization exists.

@jancborchardt
Copy link
Contributor

You want the chevron divider in this case? Would be inconsistent with the other crumb levels where you decided not to show the chevron for the last crumb, no?

Ah sorry of course you are right, no chevron on last one, also cause it would look weird with the dropdown arrow right before, too many arrows. :)

@raimund-schluessler raimund-schluessler force-pushed the fix/noid/crumb-actions-placement branch 6 times, most recently from 2e3d15a to 2cb3682 Compare January 5, 2022 15:13
@raimund-schluessler
Copy link
Contributor Author

Allow done. Checkout the video in #2416 (comment) and play around with the docs on netlify: https://deploy-preview-2416--nextcloud-vue-components.netlify.app/#/Components/Breadcrumbs?id=breadcrumbs-1

@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 5, 2022
Copy link
Contributor

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice, just 2 things found in testing:

  • Currently the text is colored using color-main-text and opacity: .5 – instead normally it should be color-text-maxcontrast and hover/focus color-main-text.
  • When in a subfolder, the home icon needs hover/focus feedback as well as a clicky-cursor because then it’s actually clickable (unlike when you are in the root folder). If it makes it easier, it’s fine if this is the same behavior in root, although it just reloads the same view (could be seen as alternate way of "refresh".

@raimund-schluessler raimund-schluessler force-pushed the fix/noid/crumb-actions-placement branch 2 times, most recently from 2cb3682 to 8827156 Compare January 6, 2022 13:10
@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Jan 6, 2022

  • Currently the text is colored using color-main-text and opacity: .5 – instead normally it should be color-text-maxcontrast and hover/focus color-main-text.

Done.

  • When in a subfolder, the home icon needs hover/focus feedback as well as a clicky-cursor because then it’s actually clickable (unlike when you are in the root folder). If it makes it easier, it’s fine if this is the same behavior in root, although it just reloads the same view (could be seen as alternate way of "refresh".

True, that was a fallacy of mine. It should be the last crumb that doesn't get a hover feedback (unless it has actions). I fixed it. See the updated video in the first comment.

Copy link
Contributor

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Perfect! :)

Copy link
Contributor

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

  • misaligned
    Capture d’écran_2022-01-06_15-23-24
  • It also means we cannot click the last folder anymore @jancborchardt. Currently on files, it refreshes the view. We will lose that feature ⚠️
    no preferences from me, you decide, but you should be aware :)

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Jan 6, 2022

  • misaligned
    Capture d’écran_2022-01-06_15-23-24

That is due to #2435, so not related to this PR. But since #2435 was just merged, I will rebase, so it looks nice for you 😉

  • It also means we cannot click the last folder anymore @jancborchardt. Currently on files, it refreshes the view. We will lose that feature ⚠️
    no preferences from me, you decide, but you should be aware :)

True, that's why I didn't do it like this initially. But I think it's fine. Clicking on the crumb for refreshing the view wouldn't be my first thought, to be honest.

@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 6, 2022
@skjnldsv skjnldsv merged commit 8f4c97d into master Jan 6, 2022
@skjnldsv skjnldsv deleted the fix/noid/crumb-actions-placement branch January 6, 2022 16:41
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 New feature or request feature: breadcrumbs Related to the breadcrumbs components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants