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

Support <text> element in SVGs #2726

Closed
lriggle-strib opened this issue Oct 5, 2024 · 4 comments · Fixed by #2736
Closed

Support <text> element in SVGs #2726

lriggle-strib opened this issue Oct 5, 2024 · 4 comments · Fixed by #2736
Labels
enhancement Improved functionality good first issue Easier issue for first time contributors help wanted Contributions are especially encouraged

Comments

@lriggle-strib
Copy link
Contributor

Search Terms

svg, icons, text vs path

Problem

The icons used to represent the various types have very small letters and are harder to identify for those with poor eyesight.

Suggested Solution

Support the <text> element inside SVGs, allowing theme builders to replace the partials/icon.tsx file within their theme to use an actual text letter. Doing this allows the font/color/size to be set to something more accessible and easier to identify for those needing support.

Doing this would require updating utils/jsx.elements.ts with the following:

  • add this interface:
    export interface JsxTextElementProps
      extends JsxSvgCoreProps,
        JsxSvgStyleProps,
        JsxSvgPresentationProps {
      x?: string | number;
      y?: string | number;
    }
  • include text: JsxTextElementProps; in the IntrinsicElements interface.

Doing this will allow theme builders to update the icon paths with text elements in an included partials/icon.tsx file making the icons easier to read.

A side-by-side comparison of updated icons, manually updated in the generated javascript file.

Before After
Screenshot 2024-10-05 at 11 08 28 AM Screenshot 2024-10-05 at 11 09 49 AM

OPTIONAL addition

Update the partials/icon.tsx file within the default theme to use <text> elements by default.

The icon for Module would go from:

    [ReflectionKind.Module]() {
        return kindIcon(
            <path
                d="M9.162 16V7.24H10.578L11.514 10.072C11.602 10.328 11.674 10.584 11.73 10.84C11.794 11.088 11.842 11.28 11.874 11.416C11.906 11.28 11.954 11.088 12.018 10.84C12.082 10.584 12.154 10.324 12.234 10.06L13.122 7.24H14.538V16H13.482V12.82C13.482 12.468 13.49 12.068 13.506 11.62C13.53 11.172 13.558 10.716 13.59 10.252C13.622 9.78 13.654 9.332 13.686 8.908C13.726 8.476 13.762 8.1 13.794 7.78L12.366 12.16H11.334L9.894 7.78C9.934 8.092 9.97 8.456 10.002 8.872C10.042 9.28 10.078 9.716 10.11 10.18C10.142 10.636 10.166 11.092 10.182 11.548C10.206 12.004 10.218 12.428 10.218 12.82V16H9.162Z"
                fill="var(--color-text)"
            />,
            "var(--color-ts-module)",
        );
    },

to something like this:

  [ReflectionKind.Module]() {
    return kindIcon(
      <text
        fill="var(--color-text)"
        x="27%"
        y="73%"
      >
        M
      </text>,
      'var(--color-ts-module)',
    );
  },

Adding in additional properties on the text element for font size, color (independent of the base text color of the page), etc would allow users are able to make the icons as readable and useful as possible without having to rebuild their own icons. Likely doing that would also require the x/y values on the text elements to be configurable, so there's stuff to think about in there.

@lriggle-strib lriggle-strib added the enhancement Improved functionality label Oct 5, 2024
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 6, 2024

Update the partials/icon.tsx file within the default theme to use <text> elements by default.

I think this is a good idea,

Likely doing that would also require the x/y values on the text elements to be configurable, so there's stuff to think about in there.

At that point, just replace the icons, it's not that much to do, and maintaining that many knobs isn't reasonable, since every small change tends to become a breaking change.

@Gerrit0 Gerrit0 added good first issue Easier issue for first time contributors help wanted Contributions are especially encouraged labels Oct 6, 2024
@lriggle-strib
Copy link
Contributor Author

@Gerrit0 I've got some code working locally that I can make a PR for, at least for an initial pass of this. It took a little while to figure out how the icons were generated, so there might be additional tasks to do around this that I'm not aware of.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 8, 2024

It should be as simple as updating the icons in icon.tsx, so long as the ids are the same other references to it should just work

@lriggle-strib
Copy link
Contributor Author

lriggle-strib commented Oct 8, 2024

That's good to know. In the branch I'm working on, the output of each icon looks like this:
Screenshot 2024-10-08 at 10 42 11 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improved functionality good first issue Easier issue for first time contributors help wanted Contributions are especially encouraged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants