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

The hamburger menu icon is visible when empty #27

Closed
sanlifer opened this issue Apr 10, 2018 · 10 comments
Closed

The hamburger menu icon is visible when empty #27

sanlifer opened this issue Apr 10, 2018 · 10 comments
Assignees

Comments

@sanlifer
Copy link

sanlifer commented Apr 10, 2018

The hamburger menu is visible in the bottom bar even though it doesn't contain any items.

Where there's only one action such as "comment", the menu icon should not be displayed.

Steps to reproduce

  1. Go to /purchase/deviations/invoices/errors/agresso-errors
  • Find an invoice with status "waiting booking receipt"
  • Click on the menu icon
    -->No result (slight color change indicates that the element reacts to the click)
@sanlifer
Copy link
Author

sanlifer commented May 2, 2018

This is still the case:
administration/groups/list-queue#administration-groups-list-queue-tab the hamburger icon is unclickable. If the menu does not contain any items it should not be displayed at all.

@dotpointer
Copy link
Contributor

Assigning some people on request by sanlifer, first one to take issue can unassign the rest or something :)

@dotpointer
Copy link
Contributor

dotpointer commented May 2, 2018

For agresso-errors I have tracked it down to cosmoz-bottom-bar. menuElements gets filled with one element:

This is what it is filled with:
<dom-repeat filter="filterSimpleActions" as="action" slot="bottom-bar-menu" tabindex="-1" class="cosmoz-bottom-bar-menu" style="display: none;"><template is="dom-repeat"></template></dom-repeat>

@JockeCK
Copy link
Contributor

JockeCK commented May 2, 2018

I am thinking(and tested locally) that we could change the row
this._setHasMenuItems(menuElements.length > 0);
into
this._setHasMenuItems(menuElements.length > 1 || this.hasExtraItems);
hasExtraItems checks for nodes in "extraSlot" which if I understand correctly,
should cover when single button "comment" for example gets hidden in an mobile view.

not 100% sure though this covers everything, I tested it locally and it worked. But resizing from mobile view into desktop view did require an browser reload to reevaluate the menu(hamburger) button into normal button again.

@JockeCK
Copy link
Contributor

JockeCK commented May 3, 2018

Actually I have changed my mind after some testing with bottom-bar demo. It worked as it should there far as I could see.

I now think the real problem is the dom-repeat, Robert posted above. So the problem is in view, not the bottom-bar component.

Closing this issue and will start a new one in the proper place later.

@JockeCK JockeCK closed this as completed May 3, 2018
@JockeCK JockeCK reopened this May 3, 2018
@JockeCK
Copy link
Contributor

JockeCK commented May 3, 2018

Pull request # 28 created.

@JockeCK
Copy link
Contributor

JockeCK commented May 3, 2018

PR #28 merged, should work now. closing

@JockeCK
Copy link
Contributor

JockeCK commented May 8, 2018

Reopening issue due to there still exists situations where this bug can occur.

Basically we need to expand the filtering we do on bottom bar node collection.

Created PR #29

JockeCK added a commit that referenced this issue May 14, 2018
Bottom-Bar: missed case where dom-if gets counted. refs #27 

Will create PR for master&next for a new minor version number. This so this fix can be tested further.
@nomego
Copy link
Contributor

nomego commented Jun 18, 2018

Fixed @JockeCK ?

@JockeCK
Copy link
Contributor

JockeCK commented Nov 9, 2018

Whoops, forgot about this one. closing

@JockeCK JockeCK closed this as completed Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants