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

feat(menu): support descriptions in vertical menu #2493

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

prudho
Copy link
Contributor

@prudho prudho commented Oct 11, 2022

Description

This PR implements a description in the menu items, which can be right floated or under a text.

Under description can also be displayed in not vertical menu, but right floated no as it looks bloated right now (see screenshots).

Screenshot

image

Inverted works too:
image

Description under text works as intended, but not the right floated one. Help is welcome 😃
image

Closes

#2232

@prudho prudho added type/feat Any feature requests or improvements lang/css Anything involving CSS state/awaiting-reviews Pull requests which are waiting for reviews state/awaiting-docs Pull requests which need doc changes/additions state/on-hold Issues and pull requests which are on hold for any reason labels Oct 11, 2022
@prudho prudho added this to the 2.9.x milestone Oct 11, 2022
@prudho prudho requested review from lubber-de and a team October 11, 2022 14:13
@prudho prudho marked this pull request as draft October 11, 2022 14:42
@prudho prudho removed the state/on-hold Issues and pull requests which are on hold for any reason label Oct 12, 2022
@lubber-de
Copy link
Member

lubber-de commented May 14, 2023

@prudho What's the status of this PR?
Regarding the issue of a description inside a non vertical item: It seems to be solved when the description node is simply declared after the text node 🙂 , no need to adjust your PR (missing related docs should provide an example to make this clear)

https://jsfiddle.net/lubber/8o5x3syq/

@lubber-de
Copy link
Member

lubber-de commented May 14, 2023

If we yould make this the standard config (first text node, then description) also for vertical item, you just need to change "flex-direction" to "column" instead of "column-reverse".
Would be more consistent

.ui.menu > .item.vertical {
  display: flex;
  flex-direction: column;
}

See
https://jsfiddle.net/lubber/8o5x3syq/2/

@lubber-de
Copy link
Member

lubber-de commented May 14, 2023

Ok i think you made it the same way it is done in dropdown menu items (first description, then text). This could be solvable there as well by setting display: flex; to dropdown menu item....BUT that would be a breaking change and people have to adjust the dropdown code, not worth the hassle
So we might keep the PR as it is and provide a docs info as suggested in my previous comment

@ko2in
Copy link
Member

ko2in commented May 15, 2023

I suggest to hold this PR for a while. I've already made a change for vertical menu as a flex item in my current branch based on my last PR #1838.

I'm still in progress of fixing merge conflicts since there were too many commits ahead of my PR.

@lubber-de lubber-de added state/on-hold Issues and pull requests which are on hold for any reason and removed state/awaiting-reviews Pull requests which are waiting for reviews labels May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/css Anything involving CSS state/awaiting-docs Pull requests which need doc changes/additions state/on-hold Issues and pull requests which are on hold for any reason type/feat Any feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants