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

Is width="100%" height="100%" not considered as default attributes on <svg> ? #1262

Closed
sambegin opened this issue Jan 23, 2020 · 13 comments
Closed

Comments

@sambegin
Copy link

sambegin commented Jan 23, 2020

Bug

We're using this library along with react-native-transformer to load our .svg files directly as react components. We recently added a compression step with SVGO library to reduce our .svgs files. By default, this library removes default attributes. One of them is the width and height on the tag that has 100% (it looks like it should be 100% by default according to MDN. Unfortunately, by removing those two attributes, our svgs are not displayed anymore.

Is this something expected?

I don't know if that can help but I also found out that the result svg in react native debugger was :
Screen Shot 2020-01-23 at 5 43 38 PM

and by forcing the width and height to 100%:
Screen Shot 2020-01-23 at 5 44 10 PM

our svgs were displayed again.

Environment info

React native info output:

System:
    OS: macOS Mojave 10.14.6
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
    Memory: 428.48 MB / 16.00 GB
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 12.13.1 - ~/.nvm/versions/node/v12.13.1/bin/node
    npm: 6.12.1 - ~/.nvm/versions/node/v12.13.1/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 12.4, macOS 10.14, tvOS 12.4, watchOS 5.3
    Android SDK:
      API Levels: 26, 27, 28, 29
      Build Tools: 28.0.2, 28.0.3, 29.0.0, 29.0.2
      System Images: android-26 | Google Play Intel x86 Atom, android-28 | Google Play Intel x86 Atom
  IDEs:
    Android Studio: 3.5 AI-191.8026.42.35.5791312
    Xcode: 10.3/10G8 - /usr/bin/xcodebuild

Library version: 11.0.1

Steps To Reproduce

  1. Import an Svg file with that has the width and height attribute on the svg tag with 100% value
    import MySVG from '../../mySVGFile.svg'
    and use it as is in any component : <MySVG />
  2. Remove the width="100%" and height="100%"

Describe what you expected to happen:

  1. I would expect the svg to be still displayed as per the default spec of svgs
@msand
Copy link
Collaborator

msand commented Feb 29, 2020

This should probably be fixed yeah, can add it to defaultProps of Svg

msand added a commit that referenced this issue Mar 4, 2020
BREAKING CHANGE: default size might change if width or height is missing
msand pushed a commit that referenced this issue Mar 4, 2020
# [12.0.0](v11.0.1...v12.0.0) (2020-03-04)

* fix!: #1262 default width and height on svg ([1d6798b](1d6798b)), closes [#1262](#1262)
* fix!: #1247 Animated view translation inside Svg tag ([0288d95](0288d95)), closes [#1247](#1247) [#1258](#1258)

### Bug Fixes

* **ios:** handle gradient and pattern transform when null ([715e9b8](715e9b8))
* **ios:** pod install error ([675df92](675df92))
* **web:** [#1274](#1274) Unable to build using babel-plugin-react-native-web ([80b5064](80b5064))
* removed missing unnecessary React headers import error caused by non-framework style import ([f795029](f795029))

### Performance Improvements

* optimize extraction of fill, stroke, responder, matrix & display ([279c3fc](279c3fc))
* optimize handling of font properties in G elements ([0fa4177](0fa4177))
* optimize handling of inherited styles ([363c1b4](363c1b4))
* optimize svg root prop handling, simplify element development ([f0cd11d](f0cd11d))

### BREAKING CHANGES

* default size might change if width or height is missing
* Behavior of native elements is reverted to pre v10
@msand msand closed this as completed Mar 4, 2020
@zoontek
Copy link
Contributor

zoontek commented Mar 5, 2020

This change breaks sizing the svg using StyleSheet.

For example:

Screenshot 2020-03-05 at 10 54 59

Will render with a height & width set at 100%. Isn't it supposed to be 14 & 14? Or should we always use height and width props?

@msand
Copy link
Collaborator

msand commented Mar 5, 2020

@zoontek Can you try the latest commit from the develop branch?

msand pushed a commit that referenced this issue Mar 5, 2020
## [12.0.1](v12.0.0...v12.0.1) (2020-03-05)

### Bug Fixes

* [#1262](#1262) allow setting width and height using stylesheet ([c5374b2](c5374b2))
* react-native 0.59 compat ([c4dba22](c4dba22))
@msand
Copy link
Collaborator

msand commented Mar 5, 2020

🎉 This issue has been resolved in version 12.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@msand msand added the released label Mar 5, 2020
@zoontek
Copy link
Contributor

zoontek commented Mar 5, 2020

@msand It still does not work correctly.

<Svg viewBox="0 0 200 200" style={{ backgroundColor: "red", aspectRatio: 1, width: "50%" }}>
  <G fill="#FFF">
    <Path d="M120.95 79.05V67.62h11.43v11.42h-11.43z" />
    {/* … */}
  </G>
</Svg>

Will result in:

Image from iOS

The SVG is wider than 50%.

@zoontek
Copy link
Contributor

zoontek commented Mar 5, 2020

Too bad this change was made, by default Image in React Native / React Native Web doesn't have any size too and we have too set a size to make them appear.

The real issue was with the SVGO config IMHO, removing consistency between <Svg> and <Image> seems a bit like a regression…

EDIT: I confirm, 12.0.1 does not solve it 😕

@msand
Copy link
Collaborator

msand commented Mar 5, 2020

Perhaps the default size should only be applied if both width and height are missing? Wasn't even aware of the single dimension + aspectRatio support. Thanks for bringing this up

@zoontek
Copy link
Contributor

zoontek commented Mar 5, 2020

It would be easier to understand to keep default React Native behavior (no size).

SVGO has a removeDimensions option that can be disabled.

@msand
Copy link
Collaborator

msand commented Mar 5, 2020

Well, this is more about aligning with the svg spec, which does specify an initial value of auto, which in the context of an svg element is treated as 100%
https://svgwg.org/svg2-draft/geometry.html#Sizing

@msand
Copy link
Collaborator

msand commented Mar 5, 2020

And in the 1.1 spec: https://www.w3.org/TR/SVG11/struct.html#SVGElementWidthAttribute

width = ""
For outermost svg elements, the intrinsic width of the SVG document fragment. For embedded ‘svg’ elements, the width of the rectangular region into which the ‘svg’ element is placed.
A negative value is an error (see Error processing). A value of zero disables rendering of the element.
If the attribute is not specified, the effect is as if a value of '100%' were specified.
Animatable: yes.

height = ""
For outermost svg elements, the intrinsic height of the SVG document fragment. For embedded ‘svg’ elements, the height of the rectangular region into which the ‘svg’ element is placed.
A negative value is an error (see Error processing). A value of zero disables rendering of the element.
If the attribute is not specified, the effect is as if a value of '100%' were specified.
Animatable: yes.

@zoontek
Copy link
Contributor

zoontek commented Mar 5, 2020

I understand, but Image is not totally aligned on img too. b2f7605 might be a good compromise.

msand pushed a commit that referenced this issue Mar 5, 2020
## [12.0.2](v12.0.1...v12.0.2) (2020-03-05)

### Bug Fixes

* [#1262](#1262) support single dimension + aspectRatio syntax ([b2f7605](b2f7605))
@msand
Copy link
Collaborator

msand commented Mar 5, 2020

🎉 This issue has been resolved in version 12.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@zoontek
Copy link
Contributor

zoontek commented Mar 5, 2020

@msand Version 12.0.2 works! Thanks

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

No branches or pull requests

3 participants