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.

Sign up for GitHub

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 support for SVG tspan and image elements, and the preserveAspectRatio attribute #830

Closed
wants to merge 3 commits into from
Closed

Add support for SVG tspan and image elements, and the preserveAspectRatio attribute #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e5c556
Add support for SVG tspan and image elements, and the preserveAspectR…
ericflo Jan 7, 2014
8d8fbef
Merge branch 'master' into svg-images
ericflo Jan 7, 2014
c7009f8
Add missing image element to xjs knownTags
ericflo Jan 7, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions docs/docs/ref-04-tags-and-attributes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ title tr track u ul var video wbr
The following SVG elements are supported:

```
circle defs g line linearGradient path polygon polyline radialGradient rect stop
svg text
circle defs g image line linearGradient path polygon polyline radialGradient
rect stop svg text tspan

```

Expand Down Expand Up @@ -69,7 +69,11 @@ In addition, there is the React-specific attribute `dangerouslySetInnerHTML` ([m
### SVG Attributes

```
cx cy d fill fx fy gradientTransform gradientUnits offset points r rx ry
spreadMethod stopColor stopOpacity stroke strokeLinecap strokeWidth transform
version viewBox x1 x2 x y1 y2 y
cx cy d fill fx fy gradientTransform gradientUnits offset points
preserveAspectRatio r rx ry spreadMethod stopColor stopOpacity stroke
strokeLinecap strokeWidth transform version viewBox x1 x2 x y1 y2 y
```

The non-standard `xlinkNamespace` and `xlinkHref` attributes are supported in
order for SVGs to be able to include linked images, and they resolve into
`xmlns:xlink` and `xlink:href`, respectively.
4 changes: 3 additions & 1 deletion src/core/ReactDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ var ReactDOM = objMapKeyVal({
rect: false,
stop: false,
svg: false,
text: false
text: false,
tspan: false,
image: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical ordering?

}, createDOMComponentClass);

var injection = {
Expand Down
7 changes: 6 additions & 1 deletion src/dom/DefaultDOMPropertyConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ var DefaultDOMPropertyConfig = {
gradientUnits: MUST_USE_ATTRIBUTE,
offset: MUST_USE_ATTRIBUTE,
points: MUST_USE_ATTRIBUTE,
preserveAspectRatio: MUST_USE_ATTRIBUTE,
r: MUST_USE_ATTRIBUTE,
rx: MUST_USE_ATTRIBUTE,
ry: MUST_USE_ATTRIBUTE,
Expand All @@ -144,6 +145,8 @@ var DefaultDOMPropertyConfig = {
transform: MUST_USE_ATTRIBUTE,
version: MUST_USE_ATTRIBUTE,
viewBox: MUST_USE_ATTRIBUTE,
xlinkNamespace: MUST_USE_ATTRIBUTE,
xlinkHref: MUST_USE_ATTRIBUTE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I don't think this works. If React tries to change the values, it would have to use setAttributeNS, right?

element.setAttributeNS(namespace,name,value)

Support for namespaced attributes in general has not really been tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm you may be right, I only tested that it properly creates the image in the first place -- didn't try changing the value once it was rendered.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericflo Any update on namespaced attributes, did they work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't gotten a chance to work on it. In fact, I ran into enough issues with SVGs that I had to actually just bypass React for now for this part of the project I'm working on.

x1: MUST_USE_ATTRIBUTE,
x2: MUST_USE_ATTRIBUTE,
x: MUST_USE_ATTRIBUTE,
Expand All @@ -161,7 +164,9 @@ var DefaultDOMPropertyConfig = {
stopOpacity: 'stop-opacity',
strokeLinecap: 'stroke-linecap',
strokeWidth: 'stroke-width',
viewBox: 'viewBox'
viewBox: 'viewBox',
xlinkNamespace: 'xmlns:xlink',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not xmlnsXlink? xlinkNamespace seems like a weird name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to change it to xmlnsXlink if that's better.

xlinkHref: 'xlink:href'
},
DOMPropertyNames: {
autoCapitalize: 'autocapitalize',
Expand Down
7 changes: 5 additions & 2 deletions src/vendor/core/getMarkupWrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ var shouldWrap = {
'radialGradient': true,
'rect': true,
'stop': true,
'text': true
'text': true,
'tspan': true
};

var selectWrap = [1, '<select multiple="true">', '</select>'];
Expand Down Expand Up @@ -79,6 +80,7 @@ var markupWrap = {
'circle': svgWrap,
'defs': svgWrap,
'g': svgWrap,
'image': svgWrap,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should image be also in shouldWrap with other svg elements?

'line': svgWrap,
'linearGradient': svgWrap,
'path': svgWrap,
Expand All @@ -87,7 +89,8 @@ var markupWrap = {
'radialGradient': svgWrap,
'rect': svgWrap,
'stop': svgWrap,
'text': svgWrap
'text': svgWrap,
'tspan': svgWrap
};

/**
Expand Down
2 changes: 2 additions & 0 deletions vendor/fbtransform/transforms/xjs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ var knownTags = {
html: true,
i: true,
iframe: true,
image: true,
img: true,
input: true,
ins: true,
Expand Down Expand Up @@ -142,6 +143,7 @@ var knownTags = {
title: true,
tr: true,
track: true,
tspan: true,
u: true,
ul: true,
'var': true,
Expand Down