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

Windows previews improvements #1854

Merged
merged 4 commits into from
Feb 22, 2023

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Oct 15, 2022

Various cleanups and improvements to the window previews, and finally fixing a very-old bug that was causing the previews not to be properly aligned.

…aling in account

Even if St.Theme is supposed to scale down all the css values, this
doesn't seem to happen for the menus max-width / height values and so
we may end up having too big menus to be visible.

LP: #1992980
… menu

It's not an object property, so no need to keep it changed here.
Sadly this is not just a style change, because apparently St is bugged
when it comes to handle the first/last child properties so if we'd just
use normal allocation we would have lost the special padding for those
elements causing unwanted behavior.

In this way, instead we preserve the style-defined sizes also for
first/last children.
Comment on lines +385 to +397
vfunc_style_changed() {
super.vfunc_style_changed();

// For some crazy clutter / St reason we can't just have this handled
// automatically or here via vfunc_allocate + vfunc_get_preferred_*
// because if we do so, the St paddings on first / last child are lost
const themeNode = this.get_theme_node();
let [minWidth, naturalWidth] = this._box.get_preferred_width(-1);
let [minHeight, naturalHeight] = this._box.get_preferred_height(naturalWidth);
[minWidth, naturalWidth] = themeNode.adjust_preferred_width(minWidth, naturalWidth);
[minHeight, naturalHeight] = themeNode.adjust_preferred_height(minHeight, naturalHeight);
this.set({ minWidth, naturalWidth, minHeight, naturalHeight });
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fmuellner Hey, do you have any clue why this ugly stuff is the only way to be able to get the size to be adjusted to use the actual padding to that is defined for first and last children?

To explain, this is basically just a PopupMenu.PopupBaseMenuItem, if from css I set something like:

       &:ltr:first-child {
            padding-left: 100px;
            padding-top: 40px;
        }
        &:ltr:last-child {
            padding-right: 100px;
        }

then, the children are positioned like if that is taken into account, but the size is not adjusted (in fact vfunc_get_natural_width is never called after the first time is set), and forcing allocating would never get the first/last items size in account.

So getting this:

image

Instead of the expected

image

Choose a reason for hiding this comment

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

Not sure whether that's the issue, but :first-child refers to this._ornament and not this._box.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, sure... Here it seems though that it's correctly computed and assigned, the problem is that the size of PopupBaseMenuItem is not adjusted when the first children is set to have some specific padding values.

It's weird, as applying something similar upstream works in all the other menu items I tried, while this one has nothing special.

@3v1n0 3v1n0 merged commit 4ce8b39 into micheleg:master Feb 22, 2023
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

Successfully merging this pull request may close these issues.

2 participants