Support all available themes

fdocr commented 3 years ago

Is your feature request related to a problem? Please describe. Now that the most recent refactor was merged there is a clear interface to validate and support all existing themes.

Describe the solution you'd like Right now we have a method that gets called whenever we need to make sure the Shell is in sync with the contents of the ForemWebView (see ensureShellState).

The current implementation supports the colors/themeing that were used before the refactor (night, hacker and default). Ideally we would make that switch statement exhaustive and support the other available themes. I think the minimal theme may work well with the same colors as the default theme, but it wouldn't hurt to optimize for it if possible. The Pink theme is what's most notably missing.

Additional context The app is currently structured in a way that it will react to changes in the ForemWebView. It will trigger the ensureShellState function whenever updates need to be made. This is why we would ideally not need to make any other changes in order to achieve this.

fdocr commented 3 years ago

@chickdan I wanted to give you the chance to take this issue since you had previously shared you would be interested in helping on this area.

No pressure or rush in having to immediately send in a PR, but if you're interested in working on this let me know and I'll assign the issue to you.

chickdan commented 3 years ago

Sure, I can work on it! I'll probably be tackling this in multiple phases depending on what I discover while researching theming. I've implemented some basic theming in my own work and will probably apply the same strategy for the first phase of this feature. However, I do want to research UIAppearance and iOS 13 theming capabilities so this might be an evolving feature.

fdocr commented 3 years ago

Thanks for the PRs @chickdan! I'll help review as soon as possible this week

forem / DEV-ios

Support all available themes #236