Skip to content
This repository has been archived by the owner on Feb 25, 2021. It is now read-only.

Added SVG support (#366) #435

Merged
merged 2 commits into from
Mar 30, 2018
Merged

Added SVG support (#366) #435

merged 2 commits into from
Mar 30, 2018

Conversation

sjroe
Copy link
Contributor

@sjroe sjroe commented Mar 29, 2018

Allows SVG to be rendered by:

  • Using createElementNS for <svg> elements
  • Replacing <blazor-component> with <g> when inserting components within an SVG tree

Addresses #366

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Mar 29, 2018

Thanks @sjroe! This looks great. I'm impressed that you made the solution this simple. You can ignore the AppVeyor build failure as it looks like a random networking issue not related to your code.

Would you be able to add a simple test component for this in the test/testapps/BasicTestApp project (in a similar way to the other test components there), plus a test that uses it in the Microsoft.AspNetCore.Blazor.E2ETest project's ComponentRenderingTest.cs file? The test only needs to check that some SVG elements were inserted into the DOM correctly (as in, they can be located via Browser.FindElement(...)).

Once there's an end-to-end (E2E) test covering this, we'd definitely be happy to merge it!

@dnfclas
Copy link

dnfclas commented Mar 29, 2018

CLA assistant check
All CLA requirements met.

@SteveSandersonMS SteveSandersonMS merged commit cdeb36d into dotnet:dev Mar 30, 2018
@SteveSandersonMS
Copy link
Member

Thanks again @sjroe - this looks totally perfect! It's now merged. Great to have this feature in :)

@sjroe sjroe deleted the svg branch March 30, 2018 17:30
@SteveSandersonMS SteveSandersonMS added this to the 0.2.0 milestone Apr 16, 2018
SteveSandersonMS pushed a commit to SteveSandersonMS/BlazorMigration that referenced this pull request Nov 27, 2018
* Added SVG support (#366)

* Added E2E tests for SVG
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants