Theme editor

[nickgrato]

Adding some updates to the color and json inputs. If the json input needs to scale anymore I would submit to the idea of breaking off into a new json theme input but I feel like for this low hanging fruit tickets this should suffice ( keeping in mind this page may be re-written in the near future as well).

• Adding default theme JSON to theme editor
• styling colors inputs
• adding titles / copy

[johnshaughnessy]

The logic inserted here is at the wrong layer. If you're trying to ship this today because you have bigger fish to fry, don't let me stop you. If you're not sure how to move it to a higher layer, then we should pair to do that so that you don't repeat this pattern for future changes you want to make to the admin panel.

[johnshaughnessy]

It feels wrong to insert logic like this here and in service-editor (for handling themes). It belongs at a higher level. It would not be difficult to move it up (to where we decide to render the themes section). It just means we won't blindly render the controls for that chunk of config variables.

That being said, I understand this was meant to be done as quickly as possible, and you're still getting familiar with the admin panel.

If this code still exists in 6 months, if there are additional incremental changes planned for the admin panel, or if what I'm saying isn't clear, then we should move this logic to a higher level. If what I'm describing is unclear, then I'm happy to chat through it.

[nickgrato]

I'm thinking this still might be good here. I have ideas on how to make this more flexible in the future. but because the button ties in so much with the inputs hooks and current value it seems logical to keep in the component for now.

[johnshaughnessy]

I don't know how to lift the currentValue out from the TextField but in theory that's how I would want to handle this. Conceptually this part is meant to be a presentational component. When I added the json validation way back when, I didn't know how to do that logic at the higher level so I just stuffed it in here. If you figure out how to handle the logic in a container component, please lift both the code you added (the button and themes specific stuff) AND the JSON validation stuff I added a while back.

But I don't mind if you don't do that now.

[nickgrato]

@johnshaughnessy here is the new theme section.

[johnshaughnessy]

I think there's state in renderLongTextInput that should be pulled out.

Specifically, I think the renderThemeSection function could call renderLongTextInput directly (instead of renderConfigurable) and handle things like the default value and the reset to default theme button.

But, it's not a big deal and I don't want to force the issue.

[johnshaughnessy]

Since you know which configurables you need to render (theme.nametag-color and theme.nametag-text-color) -- it might be better to specify them by path rather than by index.

Something like

const configurable = (path) => {
  const descriptor = getDescriptors( path.split(".")[0] ).find( ([path, desc]) => desc.path === path )[1];
  return this.renderConfigurable(path, descriptor, getConfigValue(config, path));
}

// and in place of getInput, we would write:
configurable("theme.nametag-color")
configurable("theme.nametag-text-color")
...
configurable("theme.themes")

[johnshaughnessy]

I don't know how to lift the currentValue out from the TextField but in theory that's how I would want to handle this. Conceptually this part is meant to be a presentational component. When I added the json validation way back when, I didn't know how to do that logic at the higher level so I just stuffed it in here. If you figure out how to handle the logic in a container component, please lift both the code you added (the button and themes specific stuff) AND the JSON validation stuff I added a while back.

But I don't mind if you don't do that now.