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

Migrate from stretch to taffy #940

Closed
dabreegster opened this issue Jun 10, 2022 · 5 comments
Closed

Migrate from stretch to taffy #940

dabreegster opened this issue Jun 10, 2022 · 5 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@dabreegster
Copy link
Collaborator

https://github.com/DioxusLabs/taffy is the revival of the flexbox stretch crate we use currently. Should be fairly straightforward to cutover https://github.com/a-b-street/abstreet/tree/master/widgetry/src/widgets to it. An advantage down the line could be using taffy's planned table support

@dabreegster dabreegster added the good first issue Good for newcomers label Jun 10, 2022
@alice-i-cecile
Copy link

Let us know if there are features you need or bugs that you encounter :)

@inFocus7
Copy link
Contributor

If no one's picked this up, could I be assigned? 👀
I have work done locally, just need to figure out how to test if/that widgetry acts as expected.

@dabreegster
Copy link
Collaborator Author

@inFocus7, that would be amazing, thank you for picking this up!

The easiest way to test is the widgetry_demo crate. You should be able to just cd widgetry_demo; cargo run and compare to http://play.abstreet.org/0.3.24/widgetry_demo.html. The "Show timeseries" at the top creates a scrollable popup that should check a few related things. You can also run the main A/B Street apps by following https://a-b-street.github.io/docs//tech/dev/index.html#getting-started, but I don't think this is necessary before PR review. I'll think about a list of cases that really stretch stretch today, so we can check things like the table layout:
Screenshot from 2022-06-20 08-28-22

@inFocus7
Copy link
Contributor

Hey @dabreegster!

I just put a PR up for review here. I manually tested through the widgetry_demo to make sure it acts like the one you linked. Also ran random A/B Street apps to make sure nothing is broken.

It would be a good idea to test out that table layout to feel safer with this migration. Is that a custom layout you made? Or is it part of one of the existing apps in A/B Street?


Not sure if this is a new issue (maybe with my changes), but I tried running a tests locally in url.rs and saw this error:

error[E0432]: unresolved import `crate::backend::Drawable`
  --> widgetry/src/lib.rs:36:9
   |
36 | pub use crate::backend::Drawable;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^ no `Drawable` in `backend`

@dabreegster
Copy link
Collaborator Author

Thank you for the PR! Migration is done, so I'll close this issue. I'll keep an eye on taffy and see if there's any opportunity to reduce some of the code in widgetry in the future.

Not sure if this is a new issue (maybe with my changes), but I tried running a tests locally in url.rs and saw this error:

It's not related to your changes. There's some cargo features that have to be enabled for this crate to compile, and it looks like the default one somehow got ignored by cargo test. If I run cargo test at the workspace level, things are fine. If I run inside the widgetry crate, I get the same problem. I'll have a look, thanks for the heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants