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

xilem_web: Add Rotate and Scale (CSS) transform views #621

Merged
merged 8 commits into from
Oct 10, 2024

Conversation

Philipp-M
Copy link
Contributor

This sketches/implements (parts of) the transform modifier API described in Add Affine transform to Widget trait? for xilem_web.

This currently includes rotate and scale, because there are less cases to handle for these CSS transform functions.
In rotate I think we can reduce this to just radians (there are more units for <angle>: turn, grad, deg and rad (used here) but they can all be derived from radians), and percent in scale is kinda redundant (as 200% == 2.0)), for example translate has more cases as it includes all kinds of units (like px, em, % etc.), so this may require more thought (and is thus probably better for a future PR).

This can be combined with untyped transform: ... as style, these modifier views extend the transform style (while the untyped style(..) overwrites previous set values).

The svgtoy example is updated to include these views.

@flosse flosse added the web label Oct 1, 2024
@Philipp-M Philipp-M force-pushed the xilem_web-transform-modifiers branch from 48016b8 to 6133a95 Compare October 1, 2024 17:17
Copy link
Contributor

@flosse flosse left a comment

Choose a reason for hiding this comment

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

LGTM!

Just some random thoughts:
Maybe in the future we can add types that handle different units like degrees, etc, but that's probably not where we're at here (users can use an external crate for proper unit handling). Or we could add a Radians type so that the user at least knows what is being used here, although this might be an unnecessary thing that just makes things cumbersome.

@Philipp-M Philipp-M force-pushed the xilem_web-transform-modifiers branch from 6133a95 to 8030c7b Compare October 10, 2024 17:35
@Philipp-M
Copy link
Contributor Author

Or we could add a Radians type so that the user at least knows what is being used here

I think default radians and an additional degrees (to radians) function may be better, radians are usually playing better with other related math (e.g. trigonometric functions).

Anyway, I've removed the macro style_impls, it's not 100% correct TBH, as MathML styles are not (yet) working (should work after a follow-up PR, when rustwasm/wasm-bindgen#4143 is merged though). But I think I would just accept that for now, as I don't think macros are a good choice there (jump to definition with rust-analyzer e.g.). The possibility that this API is used with MathML elements is negligible I think.
I've also added style size hints, so that we avoid unnecessary allocations when creating these modifiers...

Thanks for the review, AFAICS there weren't objections in office hours, so I guess we can start experimenting with this kind of modifier API in xilem_web (with a potential future in xilem itself), so I'll merge this.

@Philipp-M Philipp-M added this pull request to the merge queue Oct 10, 2024
Merged via the queue into linebender:main with commit a4f88b7 Oct 10, 2024
17 checks passed
@Philipp-M Philipp-M deleted the xilem_web-transform-modifiers branch October 10, 2024 17:50
github-merge-queue bot pushed a commit that referenced this pull request Oct 10, 2024
…lement` interface and document `Element::style` (#667)

I missed comment artifacts in #621, added doc-comment/test for
`SvgElement` and put the `WithStyle` on the `Element` interface bounds,
as it plays better with rust-analyzer and is more consistent with the
other API.
github-merge-queue bot pushed a commit that referenced this pull request Dec 1, 2024
…t`s (#765)

As noted in
#621 (comment),
rustwasm/wasm-bindgen#4143 is now added in
wasm_bindgen 0.2.96.

This PR updates all the dependencies of xilem_web and finally correctly
supports `style` for `MathMlElement`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants