Cosmic-Chatter / Exhibitera

Free and open-source software for building and controlling museum exhibits.
MIT License
1 stars 1 forks source link

Definition Generation contains excessively nested object that doesn't appear to be used #5

Closed alexaverill closed 2 weeks ago

alexaverill commented 3 weeks ago

I noticed when Exhibitera is generating definition files for applications its generating a pretty extraneous nested object that I don't believe are used anywhere. Example Definition file before:

{
  "a": {
    "p": {
      "p": {
        "e": {
          "a": {
            "r": {
              "a": {
                "n": {
                  "c": {
                    "e": {
                      ">": {
                        "b": {
                          "a": {
                            "c": {
                              "k": {
                                "g": {
                                  "r": {
                                    "o": {
                                      "u": {
                                        "n": {
                                          "d": {
                                            "mode": "color"
                                          }
                                        }
                                      }
                                    }
                                  }
                                }
                              }
                            }
                          }
                        }
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    }
  },
  "app": "word_cloud_input",
  "appearance": {
    "background": {
      "color": "#fff",
      "mode": "color"
    },
    "font": {
      "clear": "../_fonts/OpenSans-Regular.ttf",
      "input": "../_fonts/OpenSans-Regular.ttf",
      "prompt": "../_fonts/OpenSans-Bold.ttf",
      "submit": "../_fonts/OpenSans-Regular.ttf"
    }
  },
  "attractor": {},
  "behavior": {},
  "content": {
    "localization": {}
  },
  "exhibitera_version": 5,
  "lastEditedDate": "2024-04-22T01:23:38.458Z",
  "name": "",
  "uuid": "01dfb539-67b3-4ed6-83f1-2d3a89ab52c9"
}

Example After:

{
  "app": "media_browser",
  "exhibitera_version": 5,
  "languages": {},
  "lastEditedDate": "2024-04-26T22:52:09.077Z",
  "name": "Another Test",
  "style": {
    "background": {
      "color": "#ff1111",
      "mode": "color"
    },
    "color": {
      "filterBackgroundColor": "#bc1717",
      "filterLabelColor": "#bc5858",
      "filterTextColor": "#a05a5a",
      "titleColor": "#e32222"
    },
    "font": {
      "Lightbox_caption": "../_fonts/OpenSans-Regular.ttf",
      "Lightbox_credit": "../_fonts/OpenSans-LightItalic.ttf",
      "Lightbox_title": "../_fonts/OpenSans-Bold.ttf",
      "Title": "../_fonts/OpenSans-Bold.ttf",
      "filter_label": "../_fonts/OpenSans-Bold.ttf",
      "filter_text": "../_fonts/OpenSans-Regular.ttf"
    },
    "layout": {},
    "text_size": {}
  },
  "uuid": "e2b0c66e-ebd5-4667-b213-8abca1fdea4a"
}

I added a filter to the front end input, as well as where it get saved in case there is an instance not covered by the frontend changes.

alexaverill commented 3 weeks ago

Additionally I added some config files and definition files to the global gitignore to help avoid them being committed to source control

forceflow1049 commented 3 weeks ago

@alexaverill Thanks for this contribution—this has been a nagging problem that I haven't been able to track down.

Given that you are addressing it in the frontend (exhibitera_setup_common.js), do you think it's necessary to also address it on the backend (helper_files.py). It seems like that might introduce unexpected behavior if there is ever a valid reason to write JSON with a key of value s.

alexaverill commented 3 weeks ago

I am honestly on the fence about removing it in both places, my main thinking is that if there is a setup that isn’t using the shared code, it would be caught in the backend, and removed. It was really meant as defensive code.

I also think it’s fairly unlikely that using a key of just ‘s’ would be a common or expected use case, and if it was causing unexpected behavior it would be easy to remove.

Regardless, if you are fairly confident that the shared library is used across the board I would agree that having it in the backend is extraneous, and I can pull it out.

forceflow1049 commented 3 weeks ago

The color picker library is only used during setup and writing definitions should always use exhibitera_setup_common, so I think it is safe to enforce only on the client side. Once that is removed, I will accept the PR!

alexaverill commented 3 weeks ago

Sounds good, I just pulled it out!