When a workspace uses `debug_assertions` for conditional compilation
(*like Xilem does*), especially of the `#[cfg(debug_assertions)]` kind,
it is not sufficient to just test with it either set to `true` or
`false`. Both need testing as we don't know in advance which path
contains faulty code.
An initial response to this issue was done in #431 which added some
`--release --all-targets` steps to the CI.
This PR here improves that verification in some key ways:
* In addition to the stable toolchain, also verify MSRV.
* Only turn off the `debug_assertions` configuration option as that is
the differentiator for our testing. The full `--release` option spends
time on optimizations. It may be worth doing occasional `--release`
tests to detect bugs in code optimization, but those checks aren't worth
it as part of the `cargo clippy` / `cargo check` bundle we run most
frequently.
* No longer use `--all-targets` which has known issues as explained in
the rationale section of the CI script. Instead we duplicate the regular
steps with `cargo-hack`.
* External dependencies are always built with `debug_assertions` set to
`true` which allows for cache reuse. This is a trade-off. I think the
cache reuse performance gains are worth it for regular CI runs. However
some external dependency might e.g. sneak in a higher MSRV requirement
into a `#[cfg(not(debug_assertions))]` block. Unlikely but possible.
This is again where a less frequent `--release` check would help. Maybe
scheduled, maybe just before publishing a new version. In any case,
future work.
Additionally there is a new env variable `USING_DEBUG_ASSERTIONS` and a
presence verification script `debug_assertions.sh`. These in combination
make the CI script generic across workspaces regardless of
`debug_assertions` usage. The env variable dictates whether we will
spend the time doing the extra compiling. The bash script will detect if
the variable goes out of sync with actual usage in code.
Remaining steps
- [x] Update Textbox to use this
- [x] Add internal padding
Follow-up work:
- [ ] Restore IME support
- [ ] Upstream new result of PlainEditor (once other steps are complete)
The biggest remaining issue in this PR is that IME support is not
present.
However, I think landing this is *better* than not landing it, because:
1) If we don't land it, it's going to languish again
2) Getting IME support back can be parallelised (cc @tomcur)
3) Getting Vello 0.3.0 and Parley 0.2.0 unlocks real advantages,
including full emoji support (#420).
To be clear, my first follow-up priority will be connecting the IME back
up. I do not however think this should block on Parley 0.3.0.
Discussion in
https://xi.zulipchat.com/#narrow/channel/317477-masonry/topic/Updating.20Parley.20dependency
Also adds `MessageThunk::enqueue_message` for convenience (basically
`MessageThunk::push_message` delayed with `queueMicrotask`).
See https://github.com/slowtec/xilem-leaflet/pull/4 for a use-case of
this.
This allows zooming with the mousewheel and panning with the mid/right
mouse button.
I think it would be nice to add touch support with a pinch gesture
handler as well.
It also replaces the random color with a color selector. And makes the
stroke width range logarithmic.
And adds `type_` and `name` attributes to `HtmlInputElement`.
It was also optimized by using an `Rc<AnyDomView>` to memoize all
previously drawn lines. This way I think the bottleneck now certainly is
the browser.
Most of the change is remove the `expect` and run `cargo fix`.
For the orphan view, I've added a blanket exception. At the moment, this
only applies to String, but the
I did try using `#![no_implicit_prelude]` in that module, but there are
a lot of useful things in the implicit prelude, it was worse than the
exception.
## Changes
### Masonry
- Added new padding attribute to sized_box Masonry widget.
- 3 unit tests demonstrating how padding is used.
### Xilem
- Added `Padding` struct with convenience methods for working with
paddings.
- Updated `http_cats` example to use padding when more convenient and
fixed hack.
## Examples
```rs
sized_box(label("hello world")).padding(10.) // Equal padding on all edges
sized_box(label("hello world")).padding(Padding::top(10.)) // Padding only on top edge
sized_box(label("hello world")).padding((10., 20., 30., 40.)) // Different padding for each edge
```
## HTTP Cats
Added padding on the left in the `http_cats` example, to make it more
balanced with the right side.
<img width="912" alt="Screenshot 2024-11-10 at 16 22 52"
src="https://github.com/user-attachments/assets/ce5fd4e6-412b-46c1-9387-6886ef97e653">
## Discussion
### Rename `sized_box` to `frame`?
In swiftUI the view modifier
[`.frame()`](https://developer.apple.com/documentation/swiftui/view/frame(width:height:alignment:))
is used to change the size of a view. I think the name `frame` better
describes the purpose of `sized_box`, since it also does backgrounds,
borders (and now padding). So I wanted to suggest that `sized_box` be
renamed to `frame`.
### Add `SizedBoxExt` for better ergonomics?
Similar to
[`FlexExt`](6258856569/xilem/src/view/flex.rs (L340C11-L340C18))
and
[`GridExt`](6258856569/xilem/src/view/grid.rs (L248)),
I was thinking that a new `SizedBoxExt` could be introduced to easily
wrap any view in a `sized_box`, something like the following.
```rust
pub trait SizedBoxExt<State, Action>: WidgetView<State, Action> {
fn sized_box(self) -> SizedBox<Self, State, Action> {
sized_box(self)
}
}
```
This would allow for chaining modifiers like this:
```rust
label("Hello world")
.text_size(20.)
.sized_box() // After this padding, background, width, height can be added
.padding(20.)
```
Or even somthing more advanced like this:
```rust
label("Hello world")
.sized_box()
.padding(20.)
.background(Color::Teal)
.border(Color::Blue, 4.)
.sized_box() // By wrapping in another sized box we add another border rather than overwriting the previous one
.border(Color::Magenta, 4.)
```
I needed this in xilem_web, I could've used `Arc` as well, but I think
it's fine to add this for consistency when an `Arc` is not needed.
It's basically copy-pasta of the `Arc`. (I don't think a macro is worth
it here?). Had to fix some resulting issues in the `Templated` view (due
to ambiguity?).
---------
Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
Most of the time is spent on compiling, so might as well see all the
tests that fail.
Also the `cargo test --doc` step is moved after the screenshot failure
upload to make it clearer that the doc tests don't produce any
screenshots.
This is the first merge-able part of #606.
It allows to implement custom CSDs via window management signals and a
new `WindowHandle` widget which is useful to implement a custom
titlebar.
As [this Firefox
issue](https://bugzilla.mozilla.org/show_bug.cgi?id=1675847) is finally
fixed, we can be more close to the spec with the
`click/auxclick/contextmenu` events by using a `PointerEvent` instead of
a `MouseEvent`.
I've also noticed that the DOM interface bounds/types were out of sync
with the event types (bounds), this was corrected as well.
Additionally all the `pointer...` events were added.
And a simple `PointerMsg::position() -> Point` accessor.
This allows exceptions to be burned down on a per-crate basis, rather
than needing to be addressed across the whole codebase at once.
Additionally, this now follows completely the Linebender lint set,
except for the unexpected-cfgs. I don't think I've introduced any
behaviour changes in this PR.
If the alignment was set previously, and the `CrossAxisAlignment` was
not `Start`, the width Parley believed we were in wouldn't match the
width used for Masonry layout. This would lead to weird behaviour.
See the diff for the test images in
0a68159869
for more context
* Treat doc warnings as errors. Historically we've not done so due to
various `rustdoc` bugs giving false positives but I got it to pass
without failure right now, so perhaps better times have arrived.
* Target only `x86_64-unknown-linux-gnu` on docs.rs as we don't have any
platform specific docs.
* Properly enable example scraping for `xilem`, `xilem_core`, and
`masonry`. Fully disable it for `xilem_web` which does not have examples
as Cargo defines them.
* Pass `--all-features` at docs.rs to match our CI and reduce the
maintenance burden of manually syncing the features list.
* Enable the `doc_auto_cfg` feature for docs.rs which will show a little
tip next to feature gated functionality informing of the crate feature
flag.
* Replaced `[!TIP]` with `💡 Tip` that was inside `<div
class="rustdoc-hidden" />`. The GitHub specific tip causes a `rustdoc`
error and [didn't even render
properly](ac2ca38785/masonry/src/doc/01_creating_app.md).
* Updated `01_creating_app.md` just enough to get `rustdoc` to pass, but
that file needs a bunch of more work, as it is outdated.
The [cache action's
docs](68b3cb7503/README.md)
state:
> selecting a toolchain either by action or manual `rustup` calls should
happen before the plugin, as the cache uses the current rustc version as
its cache key
In practice we weren't affected too much, because it turns out the cache
key is also derived using all environment variables that have the prefix
`RUST`, which includes e.g. our `RUST_STABLE_VER`. So even if the key
was derived using the CI runner's pre-installed old Rust toolchain, our
env variable prevented most conflicts.
The cache docs also state that it does not cache, by cleaning the
following:
> Any files in ~/.cargo/bin that were present before the action ran (for
example rustc).
As we were installing the toolchain after the cache action, the
potential for caching the toolchain seems there. However the cache size
hasn't changed after this ordering change. Perhaps it does a simple path
check only and the pre-installed toolchain was in the same path.
In any case, better to invoke the cache action after the toolchain has
been installed, as the docs suggest.
`cargo-hack` has gained the ability to use `--exclude` along with `-p`
in [cargo-hack#258](https://github.com/taiki-e/cargo-hack/pull/258).
This allows us to simplify our CI configuration.
This renames the `ElementFlags` into `PodFlags` as these are general
enough to keep them directly in the `Pod(Mut)`.
This reduces duplicated code (and thus a potential of making errors) in
the props (that logic is now in `PodMut::drop`, `SuperElement::upcast`
and `AnyElement::replace_inner`).
The `WithModifier` trait is adjusted and only implemented on the
`View::Element` with a blanket impl of `AsMut<T>` instead (which should
also reduce boilerplate), this was mostly a necessity to avoid `impl
WithModifier` for either variants `Pod` and `PodMut` on every props
(i.e. more boilerplate).
Further all kinds of event views were fixed, as they would break
currently when the underlying element changes (e.g. via a variant change
of `OneOf`). This was actually the original planned change, but I guess
that escalated a little bit...
---------
Co-authored-by: Markus Kohlhase <mail@markus-kohlhase.de>
This PR swaps the order of the `--each-feature` and `--optional-deps`
flags.
This is the order in the newer standard, as seen in e.g.
[peniko](7940e7de0a/.github/workflows/ci.yml (L114)):
`--optional-deps --each-feature --features std`.
This helps with the goal of keeping all the CI scripts as identical as
possible.
---
This PR also adds some missing comments compared to
[vello](4c6f75f499/.github/workflows/ci.yml (L42-L44))
and removes a trailing slash to match the [style of
vello](4c6f75f499/.github/workflows/ci.yml (L247-L249))
as well.
This is a pub enum within `render_root` and is required for creating a
`RenderRootOptions` which is already exported. This is needed for people
embedding masonry in a non-Xilem way.
This PR starts with supporting specialized elements (i.e. more than just
the `Element` DOM interface), starting with boolean attributes in
`HtmlInputElement` (`checked`, `default_checked`, `disabled`,
`required`, `multiple`).
With a general `OverwriteBool` modifier which is optimized by encoding
boolean flags into a `u32`, thus not requiring allocations (compared to
the other modifiers we currently have), but is limited to max 32
overwrites, which I don't think should be a real-world limitation (i.e.
`input_el.checked().checked()...checked()` 32 times makes no sense), I
also started adding tests for this, as it juggles around with
bit-operations (and we should generally start writing more tests).
I have originally planned to feature-flag this (i.e. have a feature like
`HtmlInputElement`).
But I'd like to see how far we can go without this, I haven't yet
noticed significant (binary-size) regressions in the todomvc example
(which uses `input` elements) that justifies the worse DX that
additional features introduce.
Having features for this is also not optimal for another reason: It
changes the API (e.g. `DomNode::Props` is `props::HtmlInputElement`
instead of `props::Element`.
It also factors the element state flags (`was_created`, `in_hydration`)
into a separate `ElementFlags` which is shared within a `Modifier<M>`
struct (similar as `PodMut` or `WidgetMut` in masonry), so that we don't
have to duplicate that state in every modifier. Additionally a new flag
`needs_update` is introduced, which indicates that the element in
general needs to update any modifier, and is entirely an optimization to
avoid checking every modifier whether it has changed (not yet that
important, but when we have a lot of modifiers per element, having to
check every modifier is less efficient, it's also already *slightly*
visible in the js-framework-benchmark).
For this, it unfortunately suffers similar as #705 from the free-form
syntax by being a little bit more verbose (which may be reverted after
`arbitrary_self_types` are stable).
It should also fix#716
Previously the modifier systems had design issues i.e. bugs (non-deleted
styles/classes/attributes), and were unnecessary complex.
This aims to solve this (partly) by not using separate traits, but
concrete types and a different mechanism that is closer to how
`ElementSplice` works.
There's a few fundamental properties that composable type-based
modifiers need to support to avoid surprising/buggy behavior:
* Minimize actual changes to the underlying element, as DOM traffic is
expensive.
* Be compatible to memoization: e.g. a `Rotate` view should still be
applicable to possibly memoized transform values of the underlying
element.
* Recreation when the underlying element has changed (e.g. with a change
of variants of a `OneOf`).
To support all this, the modifier system needs to retain modifiers for
each modifier-view, and track its changes of the corresponding view.
Previously all elements were directly written and separated with markers
into a `Vec` to limit the boundaries of such views, but this had issues,
when e.g. all modifiers were deleted (e.g. clearing a `Vec` of classes),
by not reacting to this (I noticed that issue in the todomvc example
with the footer).
With this PR, the count of modifiers of a modifier-view are directly
stored either (hardcoded) in the view impl or its view state, which
cleans up the concrete modifier elements (such as `AttributeModifier`,
not including a separate `Marker` variant), and makes it less prone for
errors (and is slightly less memory-intensive).
The API to use these modifiers in modifier-views was also redesigned to
hopefully be more straight-forward/idiomatic. But as mentioned above
there's still challenges, which introduce complexity (which I'd like to
hide at least for simpler cases than these modifiers, likely in a future
PR).
All of this should now be documented in the new `modifier` module, where
now the modifiers `Attributes`, `Classes` and `Styles` reside. Other
views (like events) may also end up there...
One interesting aspect compared to the previous system is the use of a
new trait `With` for modifiers.
Instead of (roughly) `Element: WithStyle`, it works with `Element:
With<Styles>`.
This prevents all kinds of reimplementations of something like
`WithStyle` for elements.
This gets especially visible in the `one_of` module, which now can be
covered by a single blanket implementation.
Further the cargo-feature "hydration" was deleted, as it causes more
headaches to maintain than it really brings benefits (minimally less
binary size), depending on the future, it may or may not make sense to
reintroduce this.
This is a very messy, very basic skeleton of what Masonry documentation
will eventually look like.
Main points are:
- Dedicated documentation modules.
- Re-using most of the language from the RFCs.
Next steps are:
- Flesh out the Widget documentation.
- Rewrite all those docs in a less placeholder-y way.
- Add chapter about the widget arena.
- Spread out that pass documentation to the respective pass files.
- Rewrite ARCHITECTURE.md.
- Add screenshots.
Fixes#376 and #389.
---------
Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
See discussion here https://github.com/linebender/xilem/pull/663 and [on
zulip](https://xi.zulipchat.com/#narrow/channel/317477-masonry/topic/Improving.20docs.20for.20WidgetMut).
Basically, previous code was using `WidgetMut<MyWidget>` as a receiver.
This can be done when WidgetMut is a local type, but external users
wouldn't be able to do it. The result would be that users importing
Masonry as a dependency but copy-pasting code from one of our widgets
would get compile errors.
This PR switches to a syntax that external crates will be able to use
when declaring widgets. It's a more verbose, less readable syntax, but
it's unambiguous and doesn't require clever tricks.
We can consider switching back to WidgetMut-as-a-receiver when
`#![feature(arbitrary_self_types)]` or some equivalent gets stabilized.
See
https://github.com/rust-lang/rust/issues/44874#issuecomment-2122179688
for current progress.
Users could easily run into difficulties where animation does not work
as expected, with no obvious hints for how to debug this. Documenting
the correct usage of `on_anim_frame` should help them.
Inspired by #687, I thought some more tidying up of the log file was in
order.
The main differences are:
1) Higher-scale spans exist for key operations
2) Something is always logged for each event, but less for high-density
eventskey
3) Less is logged for high-density events
4) Key repeats are no longer treated as high density
5) We no longer do extra work if the hover and focus paths haven't
changed (which also means less logging)
This PR *does not* depend on #687
This can be toggled using the `MASONRY_TRACE_PASSES` environment
variable.
This is as-discussed in [#xilem > Scrolling is insanely
laggy](https://xi.zulipchat.com/#narrow/channel/354396-xilem/topic/Scrolling.20is.20insanely.20laggy).
There are a few overlapping issues, but this *significantly* improves
performance in debug mode.
The main pass which was problematic was the compose pass, however this
also had a significant impact on the time taken by the
accessibility/paint passes.
This only applies to the passes which traverse the entire tree, so not
the targeted passes. I also chose to exclude update_disabled and
update_stashed, as those will not necessarily happen to all widgets.
This also significantly reduces the size of the created log files - see
[#masonry > Heavy amounts of logs with large
app](https://xi.zulipchat.com/#narrow/channel/317477-masonry/topic/Heavy.20amounts.20of.20logs.20with.20large.20app).
In most cases, if you're using the log file, you will be in development,
which means that you can hopefully recreate the issue with the logging
for the passes you need enabled.
Daniel McNab