Skip to content

Add ThemeStyle and ThemeConfig classes for wrapping themes style configuration#71

Merged
erikaheidi merged 2 commits into
minicli:mainfrom
WendellAdriel:theme-style-object
May 9, 2023
Merged

Add ThemeStyle and ThemeConfig classes for wrapping themes style configuration#71
erikaheidi merged 2 commits into
minicli:mainfrom
WendellAdriel:theme-style-object

Conversation

@WendellAdriel

@WendellAdriel WendellAdriel commented May 9, 2023

Copy link
Copy Markdown
Member

In this PR I'm adding a ThemeStyle class that will behave like a DTO for the theme style configuration.
Currently, it's being used an array where the key 0 is the foreground style and 1 is the background style.
The new class is to make things easier to understand and to have better visibility of how things work.
I also refactored some test cases to take advantage of some of the Pest features.

Since the methods getStyle and setStyle are being used only in the DefaultTheme and user-created themes extend the DefaultTheme, I think that this won't impact any user-created themes.

This is also adding a ThemeConfig class that will be an object that will group the different ThemeStyle for each type of output of a theme and the addition of this class has the same purpose as the ThemeStyle one.

@WendellAdriel

WendellAdriel commented May 9, 2023

Copy link
Copy Markdown
Member Author

If this gets merged, in a future major release we can update the getDefaultColors and getThemeColors to use the ThemeStyle directly instead of arrays for the configuration if this is something that will be wanted.

return [
    'default' => [ CLIColors::$FG_WHITE ],
    'alt' => [ CLIColors::$FG_BLACK, CLIColors::$BG_WHITE ],
    ...
];
return [
    'default' => ThemeStyle::make(CLIColors::$FG_WHITE),
    'alt' => ThemeStyle::make(CLIColors::$FG_BLACK, CLIColors::$BG_WHITE),
    ...
];

We can also update it to return a ThemeConfig directly and merge them in the constructor of the default theme:

return ThemeConfig::make([
    'default' => ThemeStyle::make(CLIColors::$FG_WHITE),
    'alt' => ThemeStyle::make(CLIColors::$FG_BLACK, CLIColors::$BG_WHITE),
    ...
])

@WendellAdriel WendellAdriel changed the title Add ThemeStyle 'DTO' for wrapping style data Add ThemeStyle and ThemeConfig classes for wrapping themes style configuration May 9, 2023
@erikaheidi erikaheidi self-requested a review May 9, 2023 15:42

@erikaheidi erikaheidi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great 👍🏼 thank you!

@erikaheidi erikaheidi merged commit 8f1619f into minicli:main May 9, 2023
@WendellAdriel WendellAdriel deleted the theme-style-object branch May 9, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants