isosphere / yew-bootstrap

Bootstrap 5 Components for the Yew Framework
https://crates.io/crates/yew-bootstrap
MIT License
36 stars 18 forks source link

Icons #29

Closed alexkazik closed 10 months ago

alexkazik commented 1 year ago

I didn't want to write all the icons by hand and created some functions in app to generate all icons as code. Now I've moved it to it's own crate: https://github.com/alexkazik/yew-bootstrap-icons

If you're interested I'll create a PR.

isosphere commented 1 year ago

I think this is an worthy developer UX improvement. Our current support for icons is poor; I think just this:

https://github.com/isosphere/yew-bootstrap/blob/a8b48fb046fc7bb66afdcf4d819ab02074edcdfd/packages/yew-bootstrap/src/util/include.rs#L42

... and support for a NavBar BrandIcon:

https://github.com/isosphere/yew-bootstrap/blob/a8b48fb046fc7bb66afdcf4d819ab02074edcdfd/packages/yew-bootstrap/src/component/navbar.rs#L195-L197

Which is used in a clunky way that can't be compile-time checked:

https://github.com/isosphere/yew-bootstrap/blob/a8b48fb046fc7bb66afdcf4d819ab02074edcdfd/examples/basics/src/main.rs#L21

Your approach is better.

I see some nightly hints in your code (must_use, at least); I'd like to target the stable toolchain.

Happy to accept a PR!

alexkazik commented 1 year ago

Sounds good, I'll create a PR (may take a few days). And it can be compiled with stable (all my public crates compile on stable,must_use is stable since a long time).

On the branch https://github.com/alexkazik/yew-bootstrap-icons/tree/icons all icons are shown in the rustdoc (the BI documentation adds bootstrap-icons via cdn). It's great that it's possible but should it be used? (Just check it out and run doc to see it.)

Unless you have another idea I'll place it in a icons module, and document that module (what's in my current root doc / readme).

isosphere commented 1 year ago

Sounds good, I'll create a PR (may take a few days). And it can be compiled with stable (all my public crates compile on stable,must_use is stable since a long time).

Ah, my mistake maybe; must_use is still marked as nightly only here: https://doc.rust-lang.org/std/hint/fn.must_use.html and the corresponding issue (https://github.com/rust-lang/rust/issues/94745) shows stabilization as incomplete.

On the branch https://github.com/alexkazik/yew-bootstrap-icons/tree/icons all icons are shown in the rustdoc (the BI documentation adds bootstrap-icons via cdn). It's great that it's possible but should it be used? (Just check it out and run doc to see it.)

I took a look; I don't have strong feelings about this, but I don't think I personally would use it when coding. I think I'd use my IDE to click-through to the generated file.

Unless you have another idea I'll place it in a icons module, and document that module (what's in my current root doc / readme).

I like that approach; I'm working through that documentation now.

I prefer the way patternfly_yew handles the generation. The development team generates the icons, and then they include that generated code in the project rather than requiring the user modify their build chain accordingly: https://github.com/patternfly-yew/patternfly-yew/tree/main/generator/icons

This makes the user experience much smoother; if they want to self-generate, they can, but they don't have to. No futzing about in Cargo.toml and making new binaries just to use some pretty icons.

alexkazik commented 1 year ago

I have a very first version ready at https://github.com/alexkazik/yew-bootstrap/commits/icons. Changing BrandType is included and a breaking change! I think the crate needs a cargo fmt --all (once in packages and examples), if you do that I'll base my work on that otherwise I'll base on the current (had to disable features on my IDE, like removing trailing whitespace or running fmt in order to not change everything).

All icons are available all the time, the build script takes care of that. I've only described how you can build a binary which copies the fonts to your dist directory. I can extend basic (or another example) to include it, or better create two examples, one for each case.

But since trunk can only copy files already there or created by running a binary, I don't know how to make this even easier, other than using the cdn.

isosphere commented 1 year ago

I've never been a fan of using code formatters, as I often disagree with their choices, but I'll give cargo fmt a look. I agree that a consistent formatting of code throughout the project is ideal.

alexkazik commented 1 year ago

The big advantage of cargo fmt is that it is consistent. If you don't like the default settings you can customize them a lot (https://rust-lang.github.io/rustfmt watch out for Stable: No - some are nightly only).

While I started with customize my settings I removed most of them (except that I'd like to use group_imports:One, because that's what I do anyway but it's nightly only; and the other exception is my first project which started with indent of 2 and I don't want to reformat all).

And I don't want to force you into it, it's just a suggestion because it can help a lot.