openforis / sepal_ui

wrapper for ipyvuetify widgets to unify the display of voila dashboards in the SEPAL plateform
https://map-app.up.railway.app
MIT License
13 stars 4 forks source link

why keeping json split files for the colors ? #575

Open 12rambau opened 2 years ago

12rambau commented 2 years ago

I haven't seen these changes before, and I want to submit a comment. I understand the advantage of having the style files organized in different folders, especially javascript and css... what is not totally clear for me are the JSON files. I saw that we are updating some of the properties directly after calling the objects where are going to be used, to be sure they're adapted to the current theme (as here: https://github.com/12rambau/sepal_ui/blob/a6836ec2c4e25386d2167379f950a9fbe08fc4c0/sepal_ui/mapping/map_btn.py#L30). So IMO I think that we are creating an extra step within the implementation. We have created a color Box to deal with the colors, wouldn't be useful do the same with the json files? @12rambau

_Originally posted by @dfguerrerom in https://github.com/12rambau/sepal_ui/pull/533#pullrequestreview-1076941869_

12rambau commented 1 year ago

That's an old one so I wanted to get rid of this issue. some reasoning about this:

let me know what you think

dfguerrerom commented 1 year ago

I don't get what you mean with:

css-like information are no more (so the map_btn example is not adapted anymore

Ok, let's say we want to keep them in different files, in the end that's not an issue, what I believe is making this process more complicated is instead of just from sepal_ui.frontend.styles import layer_style and use it, we are doubling efforts import json dir>read as text>update color... what about creating a .py wrapper to read .json files and styles? or just use the styles.py file?

And, yes, I remember I posted this issue mainly to the fact of having different files for the same thing, "layer", so I would say I'm positive to left all the layer related styles in the same file.