JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
16.24k stars 1.18k forks source link

Add ability to have subfolders in `composeResources/*` #4196

Closed AzimMuradov closed 1 month ago

AzimMuradov commented 9 months ago

I have a project with up to a hundred resources in multiple directories and modules. The current way of storing resources requires flat structure:

- composeResources
  - drawables
    - pic.png
    - pic2.png
    - ...
  - font
    - ...
  - ...

But I need to have a more complex hierarchy of folders:

- composeResources
  - drawables
    - foo
      - pic.png
    - pic2.png
    - bar
      - baz
        - pic.png
    - ...
  - font
    - font-family-1
      - ...
    - ...
  - ...

image

terrakok commented 9 months ago

If you want to use the Res class you are not supposed to have hierarchy inside the drawable dir. But if it is requred for your busines logic you may use the files directory with an access by raw path

natanfudge commented 4 months ago

@terrakok I believe the intention of the issue author was to add support for using the Res class with a syntax like Res.drawable.subfolder.resourceName when you have a hierarchy inside the drawable dir. I find that it's unergonomic to have to store every single drawable (potentially thousands) in one big folder.

terrakok commented 4 months ago

I understand your problem but let me explain how I see the resource organization logic (because it is adopted the android logic): 1) drawable resources are a part of the app UI. It is not images for your gallery. It is icons like a back or a settings. Or, might be, background for a screen or a logo for the splash screen. That's it. 2) If you need to have a list of photos in the app then put them to the files directory and use it via image loading libraries like the Coil.

That's how I did it about 10 years of android development. And I didn't have problems with hundreds of resources as you said.

Feel free to share your opinion here

Nohus commented 4 months ago

I understand this is how it works on Android, and I was also forced to get used to that limitation there. But surely we are not bound to follow the limitations of Android.

It is icons like a back or a settings

Yes, and I keep titlebar icons for my custom windows in icons/windows/close_button.png, graphics for UI components in ui/button.png, and feature specific icons in feature/foo.png, and they are all organized nicely.

I was able to use painterResource("icons/windows/close_button.png") for this and have everything organized and easy to find. This is impossible with readBytes, as it doesn't return a DrawableResource and it's suspending. Even if it was possible, we are back to using strings for resources, so the new resource system becomes just a worse experience than the old one, as we are not just using strings, but also have to do additional workarounds and asynchronous loading because everything has to go through readBytes. It's not a viable solution, because it's worse than the old system.

The choice becomes to either stay with the old system (which may not be possible for long), or dump all your resources together in a single directory. Keeping your organisation and using readBytes is not a real solution.

Other UI frameworks (like Qt for example) allow for nested directories in resources, for good reason. If someone tries Compose Desktop, asks why this doesn't work, is the answer "because Android can't do it either"? What does Android has to do with my desktop app.

mgroth0 commented 4 months ago

I was already using a hierarchical organization of my images before I tried out the generated accessors in this library. I was dissapointed to find out that I'd have to choose between flattening my image files into the drawable folder, which is less convenient or just sticking with raw string paths with Res.readBytes (as I currently do).

In my opinion, this feature would be significantly helpful. Some developers will come in expecting it.

terrakok commented 4 months ago

Some thoughts about the problem:
As I see, a first impression how the generated accesors should look like is something like:

Res.drawable.dir.subdir.image
Res.drawable.dir.icon
Res.drawable.background

for the folowing resources:

composeResources/
  drawable/
    background.png
    dir/
      icon.xml
      subdir/
        image.png

There are open questions: 1) what happens if a project has dir.xml icon in the drawable dir?

composeResources/
  drawable/
    background.png
    dir.xml
    dir/
      icon.xml
      subdir/
        image.png

2) there is a problem with generated code in corner cases: millions subdirs or some wierd named dirs. Because it requires to generate specificly organized code (like we do for the case with huge amount of resources already)

The problem can be fixed easy if we will generate accesors by the different logic:

composeResources/
  drawable/
    background.png
    dir.xml
    dir/
      icon.xml
      subdir/
        image.png
Res.drawable.background
Res.drawable.dir
Res.drawable.dir_icon
Res.drawable.dir_subdir_image

This save us from the over complicated code generation and give you an ability to organize your resources by subdirs. Does it work for you?

Nohus commented 4 months ago
  1. what happens if a project has dir.xml icon in the drawable dir?

I think this should result in an error.

  1. there is a problem with generated code in corner cases: millions subdirs or some wierd named dirs. Because it requires to generate specificly organized code (like we do for the case with huge amount of resources already)

Having rules on directory names would be reasonable, as we already have for file names. Limiting the number of subdirectories/items per directory seems fine as well. Surely if someone has millions of subdirectories, they are trying to do something that doesn't need the generated accessors, but are accessing the files programmatically with readBytes.

The problem can be fixed easy if we will generate accesors by the different logic:

Using Res.drawable.dir_subdir_image would mean we cannot use _ in file names, and we already can't use -, so there is no common way to represent spaces in names any more. Not a huge deal, but the . approach (or the current system) doesn't have that problem.

Unless _ are allowed in names and this results in an error:

composeResources/
  drawable/
    dir_subdir_image.xml
    dir/
      subdir/
        image.png

But then it's still unclear which _ is a directory separator and which _ is supposed to represent a space in a name.

okushnikov commented 2 months ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.