[ios] Refactor Styles configuration #10050

Merged

root merged 1 commit from ios/refactor-styles-configuration into master

2025-01-16 12:02:30 +00:00

kirylkaveryn commented

2025-01-10 15:32:54 +00:00

Member

Problem statement

The iOS codebase uses string literals to add and configure UI elements using the Styles and Themes classes.
The styles are predefined and then passed as a StyleName == String to the Renederes that apply the Style to the target view/control.

de3acf645c/iphone/Maps/Core/Theme/Core/Style.swift (L1-L4)
de3acf645c/iphone/Maps/Core/Theme/GlobalStyleSheet.swift (L3-L20)

So the developer should always use smth like that to configure the view:

view.setStyleAndApply("PressBackground")
imageView.setStyleAndApply("MWMBlack")
isoLinesButton.setStyleAndApply(enabled ? "MenuButtonEnabled" : "MenuButtonDisabled")
socialMediaHeaderLabel.setStyleAndApply("regular16:blackPrimaryText")

This is not a good solution because the string literals usage always leads to

typos
the developer should always go to the StyleSheet to check the styles list and find the proper one before usage
cognitive pressure (because the developer should think of how to correctly type a long style - like "AlertViewTextFieldContainer")
It is hard to update the style for all the project
If the style is deleted from the StyleSheet there will be no compile error. Ony the runtime crash (it is also not appreciated because it consumes more developers time to investigate the reason)

Solution

All the styles are converted to the enums that are separated by responsibility, guarantee the type safety and easy to use. Also there is some methods overliading to directly call the nested enums sheets:

lineView.setStyleAndApply(.divider) // Global sheet
view.setStyleAndApply(.pressBackground) // Global sheet
circleImageView.setStyle(.ppHeaderCircleIcon) // PlacePage sheet
headerTitleLabel.setFontStyle(.semibold18, color: .blackPrimary) //  Fonts sheet

Benefits:

Easier to use - autocomplete helps to choose the right one:
No typos
Typesafe
If the style is deleted from the list the compiler fails
The new style should always provide the style configuration

Additional notes:

This implementation does not affect the legacy ObjC codebase that uses setName: NSString methods. The new Swift style enums are Strings Raw Representable and are not accepted in the ObjC. But an ObjC UI codebase will be replaced step with Swift by step so there is no reason to sacrifice some Swift features to expose the new API to the ObjC.
Old style strings (like case black = "MWMBlack") are not changed so as not to break the styles that were used by the runtime attributes in the storyboards/xibs (that will be removed in the future too).

## Problem statement The iOS codebase uses _string literals_ to add and configure UI elements using the `Styles` and `Themes` classes. The styles are predefined and then passed as a `StyleName == String` to the `Renederes` that apply the Style to the target view/control. https://github.com/organicmaps/organicmaps/blob/de3acf645c3920b9ba9540982f48cea6352e0e74/iphone/Maps/Core/Theme/Core/Style.swift#L1-L4 https://github.com/organicmaps/organicmaps/blob/de3acf645c3920b9ba9540982f48cea6352e0e74/iphone/Maps/Core/Theme/GlobalStyleSheet.swift#L3-L20 So the developer should always use smth like that to configure the view: ```swift view.setStyleAndApply("PressBackground") imageView.setStyleAndApply("MWMBlack") isoLinesButton.setStyleAndApply(enabled ? "MenuButtonEnabled" : "MenuButtonDisabled") socialMediaHeaderLabel.setStyleAndApply("regular16:blackPrimaryText") ``` This is **not a good** solution because the `string literals` usage always leads to 1. typos 2. the developer should always go to the `StyleSheet` to check the styles list and find the proper one before usage 3. cognitive pressure (because the developer should think of how to correctly type a long style - like `"AlertViewTextFieldContainer"`) 4. It is hard to update the style for all the project 5. If the style is deleted from the `StyleSheet` there will be no compile error. Ony the runtime crash (it is also not appreciated because it consumes more developers time to investigate the reason) ## Solution All the styles are converted to the enums that are separated by responsibility, guarantee the type safety and easy to use. Also there is some methods overliading to directly call the nested enums sheets: ```swift lineView.setStyleAndApply(.divider) // Global sheet view.setStyleAndApply(.pressBackground) // Global sheet circleImageView.setStyle(.ppHeaderCircleIcon) // PlacePage sheet headerTitleLabel.setFontStyle(.semibold18, color: .blackPrimary) // Fonts sheet ``` ### Benefits: - Easier to use - autocomplete helps to choose the right one: <img width="400" alt="image" src="https://github.com/user-attachments/assets/5c8b1bcb-e67d-417f-9cf5-29309d871ee0" /> - No typos - Typesafe - If the style is deleted from the list the compiler fails - The new style should always provide the style configuration ### Additional notes: - This implementation does not affect the legacy ObjC codebase that uses `setName: NSString` methods. The new Swift style enums are `Strings Raw Representable` and are not accepted in the ObjC. But an ObjC UI codebase will be replaced step with Swift by step so there is no reason to sacrifice some Swift features to expose the new API to the ObjC. - Old style strings (like `case black = "MWMBlack"`) are not changed so as not to break the styles that were used by the runtime attributes in the storyboards/xibs (that will be removed in the future too).

biodranik (Migrated from github.com) reviewed 2025-01-10 15:32:54 +00:00

vng (Migrated from github.com) approved these changes 2025-01-14 16:17:13 +00:00

vng (Migrated from github.com) left a comment

What is .global(.constant) notation? It is not possible to use simple .constant?

What is ```.global(.constant)``` notation? It is not possible to use simple .constant?

iphone/Maps/Core/Theme/BookmarksStyleSheet.swift

vng (Migrated from github.com) commented

2025-01-14 16:08:45 +00:00

nit: Don't see a reason why "forEach + private register + switch" is better than the manual "theme.add" 4 times here and below.

kirylkaveryn reviewed 2025-01-15 12:29:30 +00:00

iphone/Maps/Core/Theme/BookmarksStyleSheet.swift

kirylkaveryn commented

2025-01-15 12:29:30 +00:00

Author

Member

maybe the private register is too much and

    allCases.forEach { style in
    switch style {
    case .titlePopularView:
      theme.add(self) { s in
        s.backgroundColor = colors.linkBlueHighlighted
        s.cornerRadius = 10
      }

will be enough

The idea is to eliminate the possibility styles adding/removing without compile checks.
The only downside is a maybe performance downgrade, but it is not very high. I've made a performance measure check and it changes from the 0.00013s to the 0.00018s

Switch guarantees that all the cases will be covered. You cannot simply delete the style without compile checking like with the manual add.theme. Manual adding is a big downside of the previous approach maybe the private register is too much and ```swift allCases.forEach { style in switch style { case .titlePopularView: theme.add(self) { s in s.backgroundColor = colors.linkBlueHighlighted s.cornerRadius = 10 } ``` will be enough The idea is to eliminate the possibility styles adding/removing without compile checks. The only downside is a maybe performance downgrade, but it is not very high. I've made a performance measure check and it changes from the 0.00013s to the 0.00018s