Skip to content

Conversation

Myestery
Copy link
Collaborator

@Myestery Myestery commented Sep 3, 2025

This pull request enhances the responsiveness of the CommandMenubar component by introducing a "compact" mode for screens with limited vertical space. When the window height is less than 700px, the menubar adapts its submenu positioning and styling to improve usability on smaller displays. The most important changes are grouped below:

Responsive Behavior and State Management:

  • Added a reactive windowHeight reference and a computed isCompactHeight property to detect when the window height is below 700px, enabling "compact" mode for the menubar. Event listeners for window resize are now managed using onMounted and onUnmounted lifecycle hooks to keep the state updated. [1] [2]

Dynamic Styling and Class Application:

  • Updated the root element's class bindings to apply the comfy-command-menu-compact class when isCompactHeight is true, enabling conditional styling for compact mode.

Compact Mode Styling:

  • Introduced new CSS rules for .comfy-command-menu-compact to force submenus to open to the right and align with the top of the menu container, ensuring better usability on short screens. Adjustments include submenu positioning, alignment for nested menus, and ensuring the submenu container uses the full available height.

/fix #5289

Screen.Recording.2025-09-04.at.04.51.59.mov

┆Issue is synchronized with this Notion page by Unito

@Myestery Myestery requested a review from a team as a code owner September 3, 2025 17:51
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 3, 2025
Copy link

github-actions bot commented Sep 3, 2025

🎭 Playwright Test Results

All tests passed across all browsers!

⏰ Completed at: 09/04/2025, 07:01:15 PM UTC

📊 Test Reports by Browser


🎉 Your tests are passing across all browsers!

@Myestery Myestery requested a review from DrJKL September 4, 2025 00:45
@DrJKL
Copy link
Contributor

DrJKL commented Sep 4, 2025

I don't like that the submenu doesn't align at all with the menu trigger here
image

The demo menus here adapt if they're too close to the edge. Are we doing something special that's overriding that behavior?

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 4, 2025
christian-byrne
christian-byrne previously approved these changes Sep 4, 2025
Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

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

LGTM.

I think the approach more aligned with the codebase would be to useBreakpoints -- and we can expect that the breakpoints will generally give us a correct idea about the height. Alternatively, we can just handle the edge case by making the menu scrollable and fill-available css properties. However, I think this solution is also fine.

@christian-byrne
Copy link
Contributor

I don't like that the submenu doesn't align at all with the menu trigger here

I agree but I think this issue was existing before this PR right?

@DrJKL
Copy link
Contributor

DrJKL commented Sep 4, 2025

I don't like that the submenu doesn't align at all with the menu trigger here

I agree but I think this issue was existing before this PR right?

An earlier version of this PR had a video that had the help menu floating at the very top.

@DrJKL
Copy link
Contributor

DrJKL commented Sep 4, 2025

In my own testing of this, the issue is mitigated by removing the !important left and top values in the unscoped style block.
I'd rather find our local overrides that are causing PrimeVue's component to behave differently than in their spec/demo and remove those.
Or we could use a @media query to apply the changes via CSS only, if we absolutely have to.

@christian-byrne
Copy link
Contributor

Tested and it solves the problem.

@christian-byrne christian-byrne merged commit da332ed into main Sep 27, 2025
19 checks passed
@christian-byrne christian-byrne deleted the fix-5289 branch September 27, 2025 02:13
christian-byrne pushed a commit that referenced this pull request Sep 27, 2025
This pull request enhances the responsiveness of the `CommandMenubar`
component by introducing a "compact" mode for screens with limited
vertical space. When the window height is less than 700px, the menubar
adapts its submenu positioning and styling to improve usability on
smaller displays. The most important changes are grouped below:

**Responsive Behavior and State Management:**

* Added a reactive `windowHeight` reference and a computed
`isCompactHeight` property to detect when the window height is below
700px, enabling "compact" mode for the menubar. Event listeners for
window resize are now managed using `onMounted` and `onUnmounted`
lifecycle hooks to keep the state updated.
[[1]](diffhunk://#diff-10ee0e60e600a168bdd45e0b9f05a148f5b9ee48f54e6c7dee737ebe7b6192e6R314-R326)
[[2]](diffhunk://#diff-10ee0e60e600a168bdd45e0b9f05a148f5b9ee48f54e6c7dee737ebe7b6192e6L103-R104)

**Dynamic Styling and Class Application:**

* Updated the root element's class bindings to apply the
`comfy-command-menu-compact` class when `isCompactHeight` is true,
enabling conditional styling for compact mode.

**Compact Mode Styling:**

* Introduced new CSS rules for `.comfy-command-menu-compact` to force
submenus to open to the right and align with the top of the menu
container, ensuring better usability on short screens. Adjustments
include submenu positioning, alignment for nested menus, and ensuring
the submenu container uses the full available height.
* 
/fix #5289




https://github.com/user-attachments/assets/e7908b57-3f07-4fc4-a119-66f2b7e398c5





┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5323-Add-compact-menu-style-for-smaller-window-heights-2636d73d3650816cb5aaf4ac76c39739)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:topbar-menu size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New menu inaccessible at smaller window sizes
3 participants