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

Add desc prop to svg #714

Closed
Aveline-art opened this issue May 11, 2022 · 3 comments · Fixed by #729
Closed

Add desc prop to svg #714

Aveline-art opened this issue May 11, 2022 · 3 comments · Fixed by #729

Comments

@Aveline-art
Copy link
Contributor

🚀 Feature Proposal

Just as titleProp allows the addition of a custom title via props, let's also make a custom desc (and its id) available as well! This will make it easier to build in accessibility into our svg components.

Motivation

I was looking at the w3 guidelines for svgs and it was recommended to have a title and description so that users can understand the meaning behind an svg.

Example

In code, something like:

Take this svg (taken from here):

<?xml version="1.0" encoding="UTF-8"?>
<svg
  width="48px"
  height="1px"
  viewBox="0 0 48 1"
  version="1.1"
  xmlns="http://www.w3.org/2000/svg"
  xmlns:xlink="http://www.w3.org/1999/xlink"
>
  <!-- Generator: Sketch 46.2 (44496) - http://www.bohemiancoding.com/sketch -->
  <title>Rectangle 5</title>
  <desc>Created with Sketch.</desc>
  <defs></defs>
  <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
    <g
      id="19-Separator"
      transform="translate(-129.000000, -156.000000)"
      fill="#063855"
    >
      <g id="Controls/Settings" transform="translate(80.000000, 0.000000)">
        <g id="Content" transform="translate(0.000000, 64.000000)">
          <g id="Group" transform="translate(24.000000, 56.000000)">
            <g id="Group-2">
              <rect id="Rectangle-5" x="25" y="36" width="48" height="1"></rect>
            </g>
          </g>
        </g>
      </g>
    </g>
  </g>
</svg>

And do something like:

import * as React from 'react'

const SvgComponent = (props) => (
  <svg width="1em" height="1em" viewBox="0 0 48 1" {...props}>
    <desc id={props.descId}>{props.desc}</desc>
    <path d="M0 0h48v1H0z" fill="currentColor" fillRule="evenodd" />
  </svg>
)

export default SvgComponent

Pitch

Making SVGs accessible benefits everyone and this will make it a lot easier to create accessible svgs.

@gregberge
Copy link
Owner

Hello @Aveline-art, I am OK with this feature, feel free to implement it!

@Aveline-art
Copy link
Contributor Author

Aveline-art commented May 14, 2022

@gregberge I would love to work on this in my free time! But I have to admit, this is my first time working with such a complex codebase. Can you let me know if my thoughts are on the right track:

  • The eventual descProp should work just the same way as the titleProp; the <title> can be included via props (after setting titleProp to true) or via cli
    • The current architecture is arranged such that the titleProp is handled via: @svgr/babel-plugin-svg-dynamic-title
    • The descProp should therefore be handled by its own module as well, perhaps @svgr/babel-plugin-svg-dynamic-desc?
    • Since the two modules are highly similiar, I should refactor out similar code between the titleProp module and the planned descProp module.
  • Once the desc module is created, I would need to edit Options to include the module
    • Since I am editing the options, I need to edit any Config variables as well the other various packages(example)
  • I also need to edit the cli
  • Once that is done, I should update tests to verify that descProp works as planned

Is there anything I missed? Or did I make some mistake somewhere? Any feedback would be appreciated as it would help me get started.

@gregberge
Copy link
Owner

Hi @Aveline-art, yeah you are totally right.

I think the easier for you is to handle the desc directly in @svgr/babel-plugin-svg-dynamic-title. Even if the name is only "title", we could precise in README that it handles "desc" + "title". You can refactor the code to reuse functions directly inside it.

When done, you can create a desc option in core, then CLI, etc..

You can add tests at each step.

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

Successfully merging a pull request may close this issue.

2 participants