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

Overflow menu opening not consistent with other flyouts #104

Open
gabbsmo opened this issue Feb 15, 2024 · 16 comments
Open

Overflow menu opening not consistent with other flyouts #104

gabbsmo opened this issue Feb 15, 2024 · 16 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed wont-fix

Comments

@gabbsmo
Copy link

gabbsmo commented Feb 15, 2024

Describe the bug
When clicking the toggle button for a flyout menu the sub menu will become visible or hidden. This does not work for the overflow button, that instead triggered by hovering.

To Reproduce
See Pattern Lab

Expected behavior
As in Fluent React, where both types of flyout menus are toggled by clicking.

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Edge
  • Version: 121
@StfBauer
Copy link
Contributor

@gabbsmo - thank you for using hTWOo.

The hover flyout is actually triggered on both interaction on hover as well as flyout.
Hover: - To show on desktop the flyout fast or if you click on it you can freeze thy flyout.
Click - Adds a special show class to the flyout to make it visible - click it again and you unlock the menu.

Screen.Recording.2024-02-15.at.15.38.23.mov

The UX in Fluent Ui on the other hand only support the click event support. Even when on desktop, where hover usually works and gives you faster access to the menu entries you always have to click. This way you don't have any quick access by simply hover over the menu.

In case of hTWOo you always have both options quick access on desktop by hover and toggle on click.

While testing on mobile I recognise that the overflow button not always fires. This should be fixed soon.

@gabbsmo
Copy link
Author

gabbsmo commented Feb 15, 2024

But the overflow flyouts are still different from the others in that sample. The others only respond to click.

@StfBauer
Copy link
Contributor

StfBauer commented Feb 15, 2024

@gabbsmo Ahhh now I see. Fair point will make the other to trigger on hover on the arrow too.

@StfBauer StfBauer added the bug Something isn't working label Feb 15, 2024
@gabbsmo
Copy link
Author

gabbsmo commented Feb 16, 2024

I found another inconsistency that unfortunately cannot be reproduced in Pattern Lab.

This is the selector for displaying the flyout on hover: .has-overflow .hoo-buttonicon-overflow:hover>.hoo-buttonflyout
This is for when clicking: .has-overflow>.hoo-overflow>.show-flyout.hoo-buttonicon-overflow>.hoo-buttonflyout

Notice that clicking requires hoo-button-icon-overflow to be a direct child of hoo-overflow. This causes issues with my existing code base where I have wrapped the button and submenu in a custom element. This causes the custom element to be the direct decendant of hoo-overflow, instead of hoo-button-icon-overflow.

@StfBauer What do you think about using something like this instead for the click activation?
.has-overflow>.hoo-overflow .show-flyout.hoo-buttonicon-overflow>.hoo-buttonflyout

@gabbsmo
Copy link
Author

gabbsmo commented Feb 16, 2024

And another one. The overflow button does not have the same height as the other command bar buttons. It looks a little out of place. You can compare with Fluent React, where there is no difference in height between overflow and other command bar buttons.

@gabbsmo
Copy link
Author

gabbsmo commented Feb 16, 2024

Sorry about the spam. Let me know if you prefer these as separate issues.

In hwtoo Pattern Lab the background changes when overing over the overflow button, but not the other buttons in the command bar. In Fluent React all buttons have a background on hover.

@StfBauer
Copy link
Contributor

@gabbsmo thank you very much for your feedback. It was awesome working with you.

I guess I fixed all the issues now for the Molecues - Command Bar with overflow

@StfBauer StfBauer reopened this Feb 21, 2024
@gabbsmo
Copy link
Author

gabbsmo commented Feb 22, 2024

Thanks @StfBauer!

I looked in Pattern Lab and the hovering and clicking seem to be consistent now.

However, if I inspect hoo-buttonicon-flyout and hoo-buttoncmd I can see that they have different height. Would be nice if hoo-buttonicon-flyout was also filling out the full height of the container, like hoo-buttoncmd.

@StfBauer
Copy link
Contributor

Now I guess we have it - 🤦‍♂️ looked at the wrong element.
https://lab.n8d.studio/htwoo/htwoo-core/?p=molecules-cmdbar-overflow

It's not yet published but if you are good with the fix I will send it out.

@gabbsmo
Copy link
Author

gabbsmo commented Feb 22, 2024

Thanks. These changes are good.

Did you look into my comment about the selector for click activated overflow flyout? I see no changes related to this.

Either way, if you want to publish, go ahead :)

@StfBauer
Copy link
Contributor

Yeah maybe I do not understand fully what you mean.

menu.hoo-cmdbar
   div.hoo-overflow
      <children>
      div.hoo-buttonicon-overflow
          button.hoo-buttonicon-overflow
          menu.hoo-buttonflyout

So this is the markup of the html - What do you like to do?

@gabbsmo
Copy link
Author

gabbsmo commented Feb 22, 2024

I am wrapping the cmdbar and its children in custom elements. Does this example make more sense?

my-toolbar
    menu.hoo-cmdbar
       div.hoo-overflow
          my-toolbar-item
              div.hoo-buttonicon-overflow.show-flyout.is-active <-- Will not work because, not direct child of .hoo-overflow, see difference in selectors for click and hover toggle in my post above -->
                  button.hoo-buttonicon-overflow
                  menu.hoo-buttonflyout

@StfBauer
Copy link
Contributor

So we are talking about web components? For that I need to have a complete code example.

@gabbsmo
Copy link
Author

gabbsmo commented Feb 22, 2024

It is AngularJS in my case.

My point is that the selector that shows the flyout on click (.has-overflow>.hoo-overflow>.show-flyout.hoo-buttonicon-overflow>.hoo-buttonflyout), will not work in my case because it says that .hoo-buttonicon-overflow must be a direct child of .hoo-overflow. In my code example above, each child is a my-toolbar-item that comes between the .hoo-overflow and .hoo-buttonicon-overflow

The selector for showing the flyout on hover (.has-overflow .hoo-buttonicon-overflow:hover>.hoo-buttonflyout) does not have this requirement.

I can try to produce a more complete code example if you still need it.

EDIT: The selectors side by side for easier comparision
.has-overflow.hoo-overflow>.show-flyout.hoo-buttonicon-overflow>.hoo-buttonflyout
.has-overflow .hoo-buttonicon-overflow:hover>.hoo-buttonflyout

@StfBauer StfBauer added help wanted Extra attention is needed wont-fix and removed bug Something isn't working labels Mar 4, 2024
@gabbsmo
Copy link
Author

gabbsmo commented Mar 4, 2024

I am a bit confused with this issue having both "help wanted" and "wont-fix". Is there anything I can do?

@StfBauer
Copy link
Contributor

StfBauer commented Mar 4, 2024

It works in any other framework, so it must be a Angular JS related one. Since I have no idea and never wrote any Angular in my life it turned out its not really a bug but has something to do with potential limitations in Angular.

I tried to loop in @chandaniprajapati on this but her time is a bit limited in the moment. You might also find help in an angular forum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed wont-fix
Projects
None yet
Development

No branches or pull requests

2 participants