Skip to content

Console icons: restrict the simple-icons fallback to an allowlist; state the icon revision's relation to the spec pin #646

Open

Description

@tnunamak

Two follow-ups from the review of #620, judged safe to merge without them.

1. Simple-icons slug collision. slugFromConnectorKey in apps/console/src/app/(console)/lib/resolve-connector-icon-simple-icons.ts lowercases the connector key and drops every non-alphanumeric character, which is how simple-icons builds its own slugs. So foo-bar, foo_bar and foobar all look up the slug foobar. A connector with no bundled or manifest mark whose key happens to match another brand's slug shows that brand's logo. This layer is the last fallback before the monogram, and #620 documents the behaviour in the doc comment but does not prevent it.

Proposal: look up a simple-icons slug only for keys on an explicit key-to-slug allowlist, and give every other key the monogram. Add a test where an unlisted key that matches a real slug renders the monogram.

2. Icon provenance does not name the spec pin. apps/console/public/connector-icons/PROVENANCE.md records the data-connectors revision the icons come from (23aee3aa). It does not say how that revision relates to the Collection Profile 0.2.0 spec pin (a87edcdc), so a reader cannot tell whether the two are meant to match or move independently. Proposal: state the relationship in PROVENANCE.md and in the comment next to EXPECTED_ASSETS in apps/console/scripts/sync-connector-icons.mjs.

Also, a wording nit from the same review: "vendored" still appears in the resolver doc comment and the sanitize-svg.ts header, though simple-icons is an npm dependency.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions