Skip to content

feat: themes#3

Open
nakajimayoshi wants to merge 17 commits intosynapticsim:masterfrom
nakajimayoshi:themes
Open

feat: themes#3
nakajimayoshi wants to merge 17 commits intosynapticsim:masterfrom
nakajimayoshi:themes

Conversation

@nakajimayoshi
Copy link
Contributor

@nakajimayoshi nakajimayoshi commented Jul 7, 2023

Closes #7

Adds support for global themes beyond light/dark modes, keeping the original theme as the default.

Philosophy:

Why not just use tailwind dark:

As of now tailwind doesn't support dynamic themes beyond light/dark mode. State-based styling adds much more flexibility and customization options.

### Why even use tailwind then with all this css?
The CSS is condensed as much as possible, with the theme-specific dynamic styles being separated from tailwind concerns into style attributes.

### Why are you passing the ThemeConfig into the redux state machine and not the UITheme?
ThemeConfig is a simpler, serializable interface that can be passed to a UITheme on state changes. The existence of UITheme serves to make the items handled by the state alterable in the future (e.g. font style, size, etc.)

Theme values overview:

--primary : Primary color for highlighting text, elements, etc.
--secondary: Secondary highlight for visual contrast
--text: main text color
--padding: color for element padding/darker elements such as cards & buttons
--workspace-padding: lighter color than padding to account for extra layers in workspace
--background: main background color

Remaining issues:

  • Reduce code duplication between tailwind config file and uiThemes.ts
  • Add selected indicator for theme menu
  • Theme selection should persist after program exit
  • Adjust styles of homepage
  • Adjust styles of workspace
  • Adjust styles of Modals
  • Should Settings have its own path or should it just be a modal?
  • Add more themes

Theme:'Default(dark)'

image
image

Theme: 'FlyByWire(dark)'

image
image

Theme: Atom-one-dark

image
image

Theme: Monokai

image
image

@nakajimayoshi

This comment was marked as resolved.

@MikeRomaa
Copy link
Member

It seems to work for me? Unless you have some uncommitted code.

@nakajimayoshi

This comment was marked as outdated.

@MikeRomaa MikeRomaa mentioned this pull request Jul 10, 2023
@nakajimayoshi nakajimayoshi marked this pull request as ready for review July 11, 2023 07:23
@IAmVisco
Copy link
Contributor

Getting an error and can't start up ACE without any theme in the local storage. || 'amethyst-dark' is not a valid JSON to parse, so it errors out.

Copy link
Member

@MikeRomaa MikeRomaa left a comment

Choose a reason for hiding this comment

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

Really liking this so far, but I'm honestly not a fan of the manually defined class names in CSS. Can we look into using one of these solutions? The package tw-colors looks really promising so maybe we can mess around with that?

@nakajimayoshi
Copy link
Contributor Author

nakajimayoshi commented Jul 11, 2023

@MikeRomaa For the first option, it will expand the color pallette quite a bit so it will take time to refactor since not only will all the class names need to change, and their invocations, but I will have to experiment with ranges of each theme color up between ranges (e.g. 100-300)

This option will be limited in the fact that each theme MUST have the same amount of color variants for each field, or the error handling will spiral out of control (e.g. you can’t have FlyByWire go up to bg-bg-200 and amethyst go up to bg-bg-300)

tw-colors seems to be more or less the same implementation here since everything is prefixed with theme, and you’d have to add additional wording to distinguish the background, text, padding, etc.

I can try shortening the classnames a bit and changing secondary to accent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Customization options and themes

3 participants