+
Skip to content

making menuitem text always vertically centered. #181

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 28, 2020
Merged

making menuitem text always vertically centered. #181

merged 1 commit into from
Aug 28, 2020

Conversation

mikeandmore
Copy link
Contributor

Currently, if a menuitem contains a miniicon, then the text isn't vertically centered. This patch fixes this strange behaviour.

@ThomasAdam
Copy link
Member

This should take into account the size of the miniicon.

@ThomasAdam ThomasAdam self-assigned this Aug 12, 2020
@mikeandmore
Copy link
Contributor Author

@ThomasAdam calc_normal_item_height() seems to suggest the menu item looks like the following:

------------------------------------------
|         gap(VerticalItemSpacing)
|-----------------------------------------
|         text
|-----------------------------------------
|         gap(VerticalItemSpacing)
------------------------------------------

No matter where you put the miniicon, the text position is always right under the gap.

Is that right?

@mikeandmore mikeandmore force-pushed the menuitem-vertical-center branch from ce8cd07 to a335e5a Compare August 27, 2020 00:20
@mikeandmore
Copy link
Contributor Author

I think I was wrong before. Yes, the height is chosen using the max of miniicon and the text + gap size.

The latest commit would fix the issue I'm encountering. The real issue is: if ItemSpacing is too large, then the text won't appear at the center.

I also tried ItemSpacing with 0 using this patch, it works well too.

@ThomasAdam
Copy link
Member

Hi @mikeandmore,

This looks much better, thanks. Last thing left to do is to expand on your commit message, please, to explain why we're making this change, and what this is fixing as a result, then I'll merge this.

@ThomasAdam ThomasAdam added this to the 1.0 milestone Aug 27, 2020
@ThomasAdam ThomasAdam added the type:enhancement Augmenting an existing feature label Aug 27, 2020
@mikeandmore
fix the vertical alignment when VerticalItemSpacing is large.
22723a8 
Previous code vertically aligns the text well when VerticalItemSpacing
is small. However, if VerticalItemSpacing is large, then the font
height + gaps can be larger than MiniIcon's height, and the text will
have an extra y_offset.
@mikeandmore mikeandmore force-pushed the menuitem-vertical-center branch from a335e5a to 22723a8 Compare August 28, 2020 18:49
@mikeandmore
Copy link
Contributor Author

How about now?

@ThomasAdam ThomasAdam merged commit 0a97ed8 into fvwmorg:master Aug 28, 2020
@mikeandmore mikeandmore deleted the menuitem-vertical-center branch November 29, 2020 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ThomasAdam ThomasAdam ThomasAdam approved these changes

Assignees

@ThomasAdam ThomasAdam

Labels

type:enhancement Augmenting an existing feature

Projects

FVWM3
Status: Done

Milestone

1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@mikeandmore @ThomasAdam
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载