Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dashboard: Better convention for supporting different assets based on theme #518

Open
koonpeng opened this issue Sep 29, 2021 · 0 comments
Labels
enhancement New feature or request

Comments

@koonpeng
Copy link
Collaborator

Feature request

Description

    "logos":{
        "headerLogo":{
            "icons":{
                "headerLogo":"/icons/headerLogo.png"
            }
        },
        "darkThemeLogo": {
            "icons":{
                "darkThemeLogo": "/icons/roshealth-logo-blue.png"
            }
        }
    }

Currently the header is defined like so. darkThemeLogo feels like an arbitrary decided key, it doesn't look like this pattern can scale to support more assets.

I propose this conventional instead:

{
  "logos": {
    "headerLogo": {
      "icons": {
        "default": {
          "src": "/icons/headerLogo.png",
          "sizes": "(max-width: 600px) 480px, 800px",
          "srcset": "srcset=\"/icons/headerLogo-480w.jpg 480w, /icons/headerLogo-800w.jpg 800w\"",
          "title": "some label"
        },
        "dark": {
          "src": "/icons/headerLogo-dark.png",
          "sizes": "(max-width: 600px) 480px, 800px",
          "srcset": "srcset=\"/icons/headerLogo-dark-480w.jpg 480w, /icons/headerLogo-dark-800w.jpg 800w\"",
          "title": "some label"
        }
      }
    }
  }
}

The default key shall be required, dark shall be optional. The fields are meant to be same as the attributes allowed in a img tag, the reason for this is so that we can easily support responsive images if needed, sometimes it is also desired for us to be able to customize the tooltip that the browser shows when you hover over an image (e.g. a company logo should show the name of the company, and not some placeholder like "headerLogo").

Note that is proposal only applies to image assets.

Implementation considerations

The proposal allows us to parse the json and use it "as is" in a img tag, this may seem like a security red flag, but I think it is fine as this json is embedded into the app through webpack, we never accept any user input to load these information at run time.

Alternatives

Additional information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant