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

MenuBar example is currently broken #596

Open
thomasjm opened this issue Jun 15, 2023 · 5 comments
Open

MenuBar example is currently broken #596

thomasjm opened this issue Jun 15, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@thomasjm
Copy link
Contributor

Description

broken-menu

Reproduce

  1. cd lumino
  2. npm install && npm run build
  3. cd examples/example-menubar
  4. npm install && npm run build
  5. google-chrome index.html (or browser of your choice)

Expected behavior

Menu should open and look normal.

Context

I suspect this is related to #432.

  • Operating System and version: Ubuntu 22.04.2
  • Browser and version: Chrome 114.0.5735.133 (Official Build) (64-bit)
  • JupyterLab version: N/A
@thomasjm thomasjm added the bug Something isn't working label Jun 15, 2023
@welcome
Copy link

welcome bot commented Jun 15, 2023

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@thomasjm thomasjm changed the title Menu example is currently broken MenuBar example is currently broken Jun 15, 2023
@fcollonval
Copy link
Member

Thanks @thomasjm

Would you like to work on a fix or do you have ideas how to fix it?

@thomasjm
Copy link
Contributor Author

I fixed it in my own Lumino project with a CSS change. This is necessary because of the change from absolute positioning to a CSS transform in #432. The CSS was essentially this:

.lm-Menu {
    left: 0px;
    top: 0px;
}

I'm guessing nobody noticed this problem before because JupyterLab etc. already have similar CSS. I think such CSS should be included directly in the Lumino CSS files so such fixes aren't needed on the user side.

@fcollonval
Copy link
Member

Thanks @thomasjm

Make sense it is similar to #595

Would you mind opening a PR to fix this?
I would enforce those style rules in the JavaScript rather than adding CSS as the JS code handles the positioning and this should not be messed with.

You can get an idea where to set top and left by looking at #432 changes.

cc @krassowski - your input will be really appreciate on this one.

@thomasjm
Copy link
Contributor Author

I would enforce those style rules in the JavaScript

I think the reason #432 stopped doing that was for DOM performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants