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

Support default-font-* properties in Live-Preview #7011

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tronical
Copy link
Member

@tronical tronical commented Dec 5, 2024

... by changing the resolution for the WindowItem to traverse the item tree from the current item, instead of going to the window.

This doesn't quite fix #4298 because rem resolution is still missing. That requires the built-in default font size function to be fixed as well, which is non-trivial.

cc #4298

@tronical tronical requested a review from ogoffart December 5, 2024 21:03
@ogoffart
Copy link
Member

ogoffart commented Dec 6, 2024

No tests?

Could you make a test that involve a PopupWindow. I have a suspicion that it may not work because the PopupWindow inject a WindowItem that doesn't define these default-font-* properties.

eg:

export component TestCase inherits Window {
   default-font-size: 140px;
   PopupWindow {
      // the font size of this text must be large
      Text { text: "hello"; } 
   }
}
``

@tronical
Copy link
Member Author

tronical commented Dec 6, 2024

No tests?

Could you make a test that involve a PopupWindow. I have a suspicion that it may not work because the PopupWindow inject a WindowItem that doesn't define these default-font-* properties.

eg:

export component TestCase inherits Window {
   default-font-size: 140px;
   PopupWindow {
      // the font size of this text must be large
      Text { text: "hello"; } 
   }
}
``

That's an excellent point - this is indeed also not handled (besides the rem issue).

@tronical tronical force-pushed the simon/window-item branch from 1aa3b07 to e7a04ef Compare March 31, 2025 08:37
@tronical
Copy link
Member Author

Rebased to resolve conflicts and to implement the suggested change to to use &ItemRc in the vtable. Tests and PopupWindow fixes are still missing.

@tronical tronical marked this pull request as draft March 31, 2025 08:37
@tronical tronical force-pushed the simon/window-item branch 2 times, most recently from e3871a1 to e298b56 Compare March 31, 2025 09:25
@tronical tronical marked this pull request as ready for review March 31, 2025 10:01
@tronical tronical force-pushed the simon/window-item branch 2 times, most recently from 463c809 to b51daf4 Compare April 1, 2025 09:27
tronical added 3 commits April 3, 2025 14:28
... by changing the resolution for the `WindowItem` to traverse the
item tree from the current item, instead of going to the window.

This doesn't quite fix #4298 because `rem` resolution is still missing.
That requires the built-in default font size function to be fixed as
well, which is non-trivial.

cc #4298
...through a component factory, as we use that in the live-preview.
@tronical tronical force-pushed the simon/window-item branch from b51daf4 to 5c4889c Compare April 3, 2025 13:02
@tronical
Copy link
Member Author

tronical commented Apr 3, 2025

PopupWindow fixes are in, as discussed with run-time resolution. rem isn't working yet as we call sp::WindowInner::from_pub(#window_adapter_tokens.window()).window_item().unwrap().as_pin_ref().default_font_size().get() instead of starting at self_rc or so. I'll keep that out of this PR for now.

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.

default-font-size and default-font-family don't work in the Live-Preview
2 participants