feat: add Svg component

[mstahv]

A very trivial implementation capable only do display static SVG at this point.

Compared to using Html, this component don't need any hacks to wrap svg into some html element.

Compared to using SVG as Image source, the SVG content displayed using this component can be styled with CSS.

Type of change

• Bugfix
• Feature

Checklist

• I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
• I have added a description following the guideline.
• The issue is created in the corresponding repository and I have referenced it.
• I have added tests to ensure my change is effective and works as intended.
• New and existing tests are passing locally with my change.
• I have performed self-review and corrected misspellings.

Additional for Feature type of change

• Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

[github-actions]

Test Results

   933 files  +1     933 suites  +1   56m 6s ⏱️ + 1m 53s
5 851 tests +7  5 815 ✔️ +7  36 💤 ±0  0 ❌ ±0 
6 105 runs  +7  6 062 ✔️ +7  43 💤 ±0  0 ❌ ±0 

Results for commit 48bf7b0. ± Comparison against base commit 7bd831f.

♻️ This comment has been updated with latest results.

[mstahv]

If any of @vaadin/flow team member or other core hackers would have good idea how to quickly implement a way to create to root element with createElementNS or another hack how to get rid of the wrapper div, it would be cleaner and good for forward compatibility (in case we ever get SVG support to Element API). Otherwise I believe this component would already help people as such.

[taefi]

Thanks @mstahv for the contribution. Just a question, why isn't using the Image helping? For example, I'm representing an SVG logo like this (and adding it directly to the layout):

Image logo = new Image("images/some-logo.svg", "logo"); // where it is located under META-INF.resources

What kind of styling you have in mind that cannot be done with Image? Or do you have a dynamic creation of svg at runtime in mind?

[mstahv]

@taefi : "Compared to using SVG as Image source, the SVG content displayed using this component can be styled with CSS."

With CSS you can for example change colors of icons without actually touching the SVG itself. @anezthes Can provide you more details.

I don't think dynamic creation of SVG itself has nothing to do with this case, that can be done with Image already if needed. This is more about styling and discoverability/documentation.

[taefi]

I suggest to rename the setters for the content of the Svg component to setContent, since having a svg variable and calling a setSvg method on it is verbose and even a little confusing.

Suggested change

	public void setSvg(String svg) {
	public void setContent(String svgContent) {

[mstahv]

I don't agree. That method name would confuse people to pass in only content of the top level svg element.

[mstahv]

In Image we have setSrc. Don't like that either, especially in that shorthand form.

[taefi]

I get your concern about a situation that someone might pass children contents of a svg instead of including the <svg ...> </svg> itself as well, but we can address that issue by a good documentation. Also, given the fact that the implementation of the validateAndSet is taking care of the presence of additional contents such as <?xml ...> or <!DOCTYPE svg... >, or not having the <svg ...> in the input, I don't see a reason to add the verbosity.

Btw, imagining myself reading/reviewing a code/documentation/tutorial (having a Svg svg = new Svg(); in the context), I still prefer:

svg.setContent("<svg ...></svg>");

over

svg.setSvg("<svg ...></svg>");

Maybe this is only my personal taste.

[taefi]

I also investigated a little about the possibility of omitting the wrapping div element, this is my findings:

• I modified the constructor to create a svg element instead of div.
• Then, modified validateAndSet to parse and set the attributes on this element and extract the internal document content of the input svg as the innerHtml instead.

    private void validateAndSet(String svgInput) {
        if (!svgInput.startsWith("<svg")) {
            // remove possible xml header & doc types
            int startIndex = svgInput.indexOf("<svg");
            if (startIndex == -1) {
                throw new IllegalArgumentException(
                        "The content don't appear to be SVG");
            }
            svgInput = svgInput.substring(startIndex);
        }
        Attributes svgAttributes = parseSvgAttributes(svgInput);
        svgAttributes.iterator().forEachRemaining(attribute -> getElement().setAttribute(attribute.getKey(), attribute.getValue()));
        String svgInnerDocument = extractSvgInnerDocument(svgInput);
        setInnerHtml(svgInnerDocument);
    }

    private Attributes parseSvgAttributes(String svgInput) {
        String svgPart = svgInput.substring(svgInput.indexOf("<svg"), svgInput.indexOf(">") + 1) + "</svg>";
        return Jsoup.parse(svgPart).getElementsByTag("svg").stream()
                .findFirst()
                .map(org.jsoup.nodes.Element::attributes)
                .orElse(new Attributes());
    }

    private String extractSvgInnerDocument(String svgInput) {
        String svgOpeningOmitted = svgInput.substring(svgInput.indexOf(">", svgInput.indexOf("<svg")) + 1).trim();
        return svgOpeningOmitted.substring(0, svgOpeningOmitted.indexOf("</svg>")).trim();
    }

The result on the browser look promising and it is rendered as:

but the problem is when it is not wrapped into a div, then the rendered svg is having a 0 width and height, even though, the rendered svg has the width and height attributes set (see the above screenshot).

I also tried setting the display: block on the <svg...> styles with no luck, but it seems that for some reason it is getting shrunk to a zero-sized box inside a Vaadin app.

Obviously, I was able to force the width: 300px; height: 200px; display: block; to the svg and its polygon content via adding a class selector, but still the svg rendering is broken for some reason:

To make sure the rendered html for the <svg> content on the browser is fine, I copied the element from developer tools into a bare html file:

<html>
<head></head>
<body>
    <svg width="300" height="200">
        <polygon points="100,10 40,198 190,78 10,78 160,198" style="fill:lime;stroke:purple;stroke-width:5;fill-rule:evenodd;"></polygon>
    </svg>
</body>
</html>

and it works completely fine:

[mstahv]

@taefi This is because Vaadin is creating the svg element on the client side with createElement, should use createElmentNS (or something to create it with the different namespace).

[mstahv]

Do I remember wrong or was there some "virtual child" concept? That with some JS hack maybe (if we don't want to touch the client side code and add an actual framework feature)?

[taefi]

@taefi This is because Vaadin is creating the svg element on the client side with createElement, should use createElmentNS (or something to create it with the different namespace).

I'm not sure if this is somehow related as well, but there seems to be an open issue for rendering <svg> in shadow dom:
WICG/webcomponents#179

[taefi]

@mstahv please format and push one last time to make it OK to merge.

Another issue for omitting the wrapper div could be created labeled as improvement.

This PR is good to be merged IMO.

[mstahv]

Formatted✅

Enhancement issue: #15601

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication