[styles] Rename styles to default/vehicle/outdoors + light/dark #7264

dvdmrtnz · 2024-01-28T19:49:34Z

The current styles naming scheme is inconsistent:

In the data/styles folder there is a clear style at the same level of the vehicle and outdoors styles, but then each of these styles is divided into clear/night, which is confusing as the same word clear is used on two different levels.

clear
├─ clear
├─ night
vehicle
├─ clear
├─ night
outdoors
├─ clear
├─ night

In the code the higher level clear style gets no name and the lower levels use clear/dark.

-
├─ clear
├─ dark
vehicle
├─ clear
├─ dark
outdoors
├─ clear
├─ dark

And then the debug commands use ?light/?dark.

--

I propose changing the naming scheme to this:

default
├─ light
├─ dark
vehicle
├─ light
├─ dark
outdoors
├─ light
├─ dark

Light/Dark is the naming convention used by both iOS (https://developer.apple.com/design/human-interface-guidelines/dark-mode) and Android (https://developer.android.com/develop/ui/views/theming/darktheme?hl=en#change-themes) in their guidelines.

This PR fixes section 1 of Global issues with icons and thoughts on improvement #4017

dvdmrtnz · 2024-01-28T19:50:52Z

@euf PTAL

euf · 2024-01-28T19:55:25Z

This is great! New naming makes a lot more sense. Thank you so much for this job, I believe it would reduce confusion for future designers a lot.

Could you please also tag me once again after this PR is merged? I would have to update a couple of issues and wiki pages which have direct links to master branch icons.

Jean-BaptisteC · 2024-01-28T19:58:57Z

Can you add git blame?

biodranik · 2024-01-28T20:03:07Z

Thanks!

How does this scheme fit adding the planned variation of the more contrast dark style that is suitable for daylight use (not only for the use at night/dark environment, as the current low-contrast style)?
Would it be possible to make a setting "use a more contrast style" to switch it (maybe also for a light theme)?
Are there any benefits of using a different structure, e.g.

light
light/default
light/outdoor
light/vehicle
dark
dark/default
dark/outdoor
dark/vehicle

?
4. Would it be possible to add a new style that is light-only or dark-only?

dvdmrtnz · 2024-01-28T21:08:44Z

Can you add git blame?

What do you want exactly? I don't understand

Jean-BaptisteC · 2024-01-28T21:16:51Z

Add your commit in .git blame ignore revs like explained here https://github.blog/changelog/2022-03-24-ignore-commits-in-the-blame-view-beta/

dvdmrtnz · 2024-01-28T21:35:55Z

Thanks!

How does this scheme fit adding the planned variation of the more contrast dark style that is suitable for daylight use (not only for the use at night/dark environment, as the current low-contrast style)?

Would it be possible to make a setting "use a more contrast style" to switch it (maybe also for a light theme)?

Are there any benefits of using a different structure, e.g.
light
light/default
light/outdoor
light/vehicle
dark
dark/default
dark/outdoor
dark/vehicle
? 4. Would it be possible to add a new style that is light-only or dark-only?

In that case, I guess a new contrast style could be added for example:

default
├─ light
├─ dark
vehicle
├─ light
├─ dark
outdoors
├─ light
├─ dark
contrast
├─ dark

If the new style for example only has dark style, a fallback in map_style_reader.cpp could be set to use the light style from the default style.

dvdmrtnz · 2024-02-03T08:55:20Z

Rebased

pastk · 2024-02-05T10:33:27Z

3. Are there any benefits of using a different structure, e.g.

The light/dark styles differ in colors only while sharing all other mapcss files. So it makes more sense to put a "style name" first and then an optional color palette.

pastk · 2024-02-05T10:36:36Z

If we go a way of different color palettes for a "night" (low-contrast) theme and a "dark" (daylight suitable) theme, then we'll be able to add a "night" subfolder, e.g.

default
├─ light
├─ dark
├─ night

But this actually means that atm we have a "light" and "night" palettes and no "dark" one.

pastk · 2024-02-05T10:41:16Z

A different thing is the current "vehicle" style.
Its used for the car navigation mode only. And we're likely to add a "car/driving" style later.

If we'd call it "driving" indeed then its OK to leave "vehicle" as is? Or maybe better to rename to e.g. "driving-navigation" as its much more clear?

pastk · 2024-02-18T07:25:05Z

Re icons folders rename.

I don't think we'd ever use different icon sets for different styles (but different colors will stay for now).
So IMO it makes sense to move them outside of the current styles/clear/style-clear/ folder.
Also its better to make separate folders for the source svg icons and many auto-generated png versions.

dvdmrtnz · 2024-03-04T16:04:50Z

Also its better to make separate folders for the source svg icons and many auto-generated png versions.

They already are in different folders. Source svg icons are in data/styles/default/light/symbols and auto-generated png versions are in data/resources-mdpi_light, resources-hdpi_light... folders.

Jean-BaptisteC · 2024-03-10T09:30:10Z

Ready to merge?

dvdmrtnz · 2024-03-10T10:39:28Z

Rebased and ready to be merged

pastk · 2024-03-11T06:34:52Z

qt/CMakeLists.txt

-  drules_proto_vehicle_clear.bin
+  drules_proto_default_light.bin
+  drules_proto_default_dark.bin
+  drules_proto_vehicle_light.bin


hmm why there is no outdoors style here?

I would guess because the desktop application has no outdoors style?

actually it has, but there is no UI for switching;
one has to type ?olight in the search - it works well

pastk · 2024-03-11T06:36:54Z

tools/python/data/styles/setup.py

+        "drules_proto_default_dark.bin",
+        "drules_proto_default_dark.txt",
+        "drules_proto_vehicle_light.bin",
+        "drules_proto_vehicle_light.txt",
        "drules_proto_vehicle_dark.bin",


also no outdoors style here
(what does this script do?)

pastk · 2024-03-11T06:37:31Z

tools/python/generate_styles_override.py

-paths = ['clear/style-clear', 'clear/style-night', 'vehicle/style-clear', 'vehicle/style-night']
-suffixes = ['_clear', '_dark', '_vehicle_clear', '_vehicle_dark']
+paths = ['default/light', 'default/dark', 'vehicle/light', 'vehicle/dark']
+suffixes = ['_default_light', '_default_dark', '_vehicle_light', '_vehicle_dark']


I don't really know what these python tools do. I just changed the styles names. I don't know why there's no outdoors styles in there

@pastk it's the question to you )

This script is used for designer tool. Since the designer is broken, we can ignore any inconsistency here

It would be great to revive designer back.

docs/STYLES.md

pastk · 2024-03-11T06:47:24Z

drape/drape_tests/static_texture_tests.cpp

-                                         MapStyle::MapStyleVehicleClear,
+  static std::vector<MapStyle> styles = {MapStyle::MapStyleDefaultLight,
+                                         MapStyle::MapStyleDefaultDark,
+                                         MapStyle::MapStyleVehicleLight,


@vng do we need the outdoors style here too?

pastk · 2024-03-11T06:48:04Z

drape_frontend/color_constants.cpp

@@ -25,7 +25,7 @@ class TransitColorsHolder
  dp::Color GetColor(std::string const & name) const
  {
    auto const style = GetStyleReader().GetCurrentStyle();
-    auto const isDarkStyle = style == MapStyle::MapStyleDark || style == MapStyle::MapStyleVehicleDark;
+    auto const isDarkStyle = style == MapStyle::MapStyleDefaultDark || style == MapStyle::MapStyleVehicleDark;


MapStyleOutdoorsDark?

pastk · 2024-03-11T07:09:56Z

Also its better to make separate folders for the source svg icons and many auto-generated png versions.

They already are in different folders. Source svg icons are in data/styles/default/light/symbols and auto-generated png versions are in data/resources-mdpi_light, resources-hdpi_light... folders.

Right, I've confused auto-generated skins with e.g. data/styles/default/light/xhdpi with manually created versions of bookmarks and search results icons..

Still, it'd be better to move data/styles/default/light/symbols/ into e.g. data/map-icons/light/svg and data/styles/default/light/xhdpi into e.g. data/map-icons/light/png/xhdpi and data/resources-mdpi_light into e.g. data/map-icons/light/skins/mdpi. As the current structure is messy and is not logical.
We can do it in a separate PR though and discuss the best (and future-proof) structure beforehand.

dvdmrtnz · 2024-03-17T09:40:50Z

Still, it'd be better to move data/styles/default/light/symbols/ into e.g. data/map-icons/light/svg and data/styles/default/light/xhdpi into e.g. data/map-icons/light/png/xhdpi and data/resources-mdpi_light into e.g. data/map-icons/light/skins/mdpi. As the current structure is messy and is not logical. We can do it in a separate PR though and discuss the best (and future-proof) structure beforehand.

I think that's a good idea. Can be done in a separate PR

pastk · 2024-04-10T20:48:55Z

@vng @biodranik could you review please?

biodranik

It is better to squash the first 4 commits into one, and remove the last one to add it after the merge.

LGTM otherwise. @vng WDYT?

biodranik · 2024-04-11T01:58:58Z

.git-blame-ignore-revs

@@ -1,3 +1,5 @@
 6aa73face8b5eb8e026cfafa40d1983d4a0502c0
 480fa6c2fcf53be296504ac6ba8e6b3d70f92b42
 a6ede2b1466f0c9d8a443600ef337ba6b5832e58
+# [styles] Move all icons to renamed styles folders
+4b39ab2413a461376b04572bc51de18e609a7b19


This hash can change after the rebase, it's safer to add this commit after the merge.

I will remove this commit and make a new PR after the merge

biodranik · 2024-04-11T02:00:15Z

drape_frontend/color_constants.cpp

@@ -25,7 +25,7 @@ class TransitColorsHolder
  dp::Color GetColor(std::string const & name) const
  {
    auto const style = GetStyleReader().GetCurrentStyle();
-    auto const isDarkStyle = style == MapStyle::MapStyleDark || style == MapStyle::MapStyleVehicleDark;
+    auto const isDarkStyle = style == MapStyle::MapStyleDefaultDark || style == MapStyle::MapStyleVehicleDark;


MapStyleOutdoorsDark?

biodranik · 2024-04-11T02:02:39Z

tools/python/generate_styles_override.py

-paths = ['clear/style-clear', 'clear/style-night', 'vehicle/style-clear', 'vehicle/style-night']
-suffixes = ['_clear', '_dark', '_vehicle_clear', '_vehicle_dark']
+paths = ['default/light', 'default/dark', 'vehicle/light', 'vehicle/dark']
+suffixes = ['_default_light', '_default_dark', '_vehicle_light', '_vehicle_dark']


@pastk it's the question to you )

dvdmrtnz · 2024-04-25T19:57:06Z

Squashed and rebased

rtsisyk · 2024-04-26T12:45:30Z

pastk: Maybe better to merge it right after the release..

dvdmrtnz · 2024-04-27T08:29:10Z

pastk: Maybe better to merge it right after the release..

Okay. Just let me know when you are ready to merge and I can rebase the branch if necessary

pastk · 2024-05-09T12:08:07Z

@dvdmrtnz as we're renaming e.g. drules_proto_vehicle_clear.bin anyway, then maybe let's make them shorter e.g. drules_vehicle_light.bin or put them together into a separate folder data/drules or maybe into respective style folders even (not sure here, probably its better to keep source and generated files separately?)
WDYT?

pastk · 2024-05-09T12:09:00Z

@vng please take a look when you have time.
It'd be great to finalize and merge it after the current May release is out.

rtsisyk · 2024-05-16T06:28:45Z

@vng please take a look when you have time. It'd be great to finalize and merge it after the current May release is out.

Please rebase and merge

clear/style-clear -> default/style-light clear/style-night -> default/style-dark vehicle/style-clear -> vehicle/style-light vehicle/style-night -> vehicle/style-dark outdoors/style-clear -> outdoors/style-light outdoors/style-night -> outdoors/style-dark Signed-off-by: David Martinez <47610359+dvdmrtnz@users.noreply.github.com>

Signed-off-by: David Martinez <47610359+dvdmrtnz@users.noreply.github.com>

Jean-BaptisteC · 2024-05-22T20:16:41Z

@rtsisyk ready to merge

dvdmrtnz requested a review from a team as a code owner January 28, 2024 19:49

dvdmrtnz force-pushed the styles branch 2 times, most recently from 41adfa7 to 5ed8714 Compare January 28, 2024 20:52

dvdmrtnz force-pushed the styles branch from 5ed8714 to ac436ef Compare January 28, 2024 21:30

dvdmrtnz force-pushed the styles branch 4 times, most recently from 6dba90a to 9e10065 Compare February 3, 2024 08:54

dvdmrtnz force-pushed the styles branch from 9e10065 to a48e346 Compare February 3, 2024 08:56

dvdmrtnz force-pushed the styles branch 2 times, most recently from a1c9b8b to 2de3e85 Compare February 17, 2024 14:45

dvdmrtnz force-pushed the styles branch from 2de3e85 to 939df7d Compare March 4, 2024 15:59

dvdmrtnz force-pushed the styles branch 2 times, most recently from d3859e3 to 56a6a9e Compare March 5, 2024 18:44

rtsisyk requested a review from a team March 10, 2024 10:33

dvdmrtnz force-pushed the styles branch from 56a6a9e to 1e9c35e Compare March 10, 2024 10:39

pastk approved these changes Mar 11, 2024

View reviewed changes

pastk requested review from vng and biodranik March 13, 2024 11:22

dvdmrtnz force-pushed the styles branch 3 times, most recently from a419cba to 32ad4e5 Compare March 17, 2024 09:29

biodranik approved these changes Apr 11, 2024

View reviewed changes

dvdmrtnz force-pushed the styles branch from 32ad4e5 to 81fcf4c Compare April 25, 2024 19:55

biodranik mentioned this pull request May 8, 2024

[data] Refactor resources folder #8123

Open

dvdmrtnz added 2 commits May 18, 2024 14:54

[styles] Move all icons to renamed styles folders

ac064ea

Signed-off-by: David Martinez <47610359+dvdmrtnz@users.noreply.github.com>

dvdmrtnz force-pushed the styles branch from 81fcf4c to ac064ea Compare May 18, 2024 13:06

vng merged commit f69f5b6 into organicmaps:master May 23, 2024
15 checks passed

dvdmrtnz deleted the styles branch May 23, 2024 08:34

dvdmrtnz mentioned this pull request May 26, 2024

Global issues with icons and thoughts on improvement #4017

Open

4 tasks

[styles] Rename styles to default/vehicle/outdoors + light/dark #7264

[styles] Rename styles to default/vehicle/outdoors + light/dark #7264

Conversation

dvdmrtnz commented Jan 28, 2024 • edited

dvdmrtnz commented Jan 28, 2024

euf commented Jan 28, 2024

Jean-BaptisteC commented Jan 28, 2024

biodranik commented Jan 28, 2024

dvdmrtnz commented Jan 28, 2024 • edited

Jean-BaptisteC commented Jan 28, 2024

dvdmrtnz commented Jan 28, 2024 • edited

dvdmrtnz commented Feb 3, 2024

pastk commented Feb 5, 2024

pastk commented Feb 5, 2024

pastk commented Feb 5, 2024

pastk commented Feb 18, 2024

dvdmrtnz commented Mar 4, 2024 • edited

Jean-BaptisteC commented Mar 10, 2024

dvdmrtnz commented Mar 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pastk commented Mar 11, 2024

dvdmrtnz commented Mar 17, 2024

pastk commented Apr 10, 2024

biodranik left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdmrtnz Apr 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdmrtnz commented Apr 25, 2024

rtsisyk commented Apr 26, 2024

dvdmrtnz commented Apr 27, 2024

pastk commented May 9, 2024

pastk commented May 9, 2024

rtsisyk commented May 16, 2024

Jean-BaptisteC commented May 22, 2024

dvdmrtnz commented Jan 28, 2024 •

edited

dvdmrtnz commented Jan 28, 2024 •

edited

dvdmrtnz commented Jan 28, 2024 •

edited

dvdmrtnz commented Mar 4, 2024 •

edited

dvdmrtnz commented Mar 10, 2024 •

edited

biodranik left a comment •

edited

dvdmrtnz Apr 25, 2024 •

edited