afonsolage / bevy_ecss

Bevy crate which uses a subset of CSS to update Bevy ECS components
Apache License 2.0
98 stars 11 forks source link

New selectors- BorderColor and Image #28

Closed Leinnan closed 9 months ago

Leinnan commented 10 months ago

Adds new selectors for setting up BorderColor and Image texture.

afonsolage commented 10 months ago

I'm fine adding non CSS properties to bevy_ecss, since it is focused on Bevy UI, not CSS itself, but I think we should somehow let users know that we have two set of properties, on which tries to keep as closes as CSS references (I'm using Mozila docs here) and another which is solely focused on Bevy itself, which can be Ui or even other utilities later on;

So IMO we should pick one of these options:

  1. Do this separation on doc level, so list properties "MSD CSS Properties" and "Bevy Properties" something like that;
  2. Do this separation on mod level, so move all "MSD CSS Properties" to something like property/css.rs and "Bevy Properties" to property/bevy.rs;
  3. Do this separation on plugin level, keep all "MSD CSS Properties" on current plugin and create another one, in case user wanna use custom Bevy properties;

I think we can do 1 and 2, which keeps code organized and also users will know what to expect from the crate.

Are are your toughts?

Leinnan commented 10 months ago

I think for some maybe it could be useful. I would see it like the keeping "MSD CSS" properties always and put all custom "Bevy properties" behind "custom_properties" feature flag that would be enabled by default but gives game makers options to even disable custom ones if they would like that.

So I also would go with first and second option, third one feels like overengineering IMHO.

Leinnan commented 10 months ago

I've created ticket for things you proposed: https://github.com/afonsolage/bevy_ecss/issues/29 https://github.com/afonsolage/bevy_ecss/issues/30 I think you could still merge it and then move on with splitting stuff as continuation of work for next release.