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

Defining the viewbox property with the SVG directive #318

Open
Vechro opened this issue Sep 14, 2020 · 6 comments
Open

Defining the viewbox property with the SVG directive #318

Vechro opened this issue Sep 14, 2020 · 6 comments

Comments

@Vechro
Copy link
Contributor

Vechro commented Sep 14, 2020

One of the only things you can't define in CSS for SVG is the viewbox property. I'd like to request a feature to pass a (perhaps optional) argument to the @svg directive that lets you define the viewbox size.

@s0kil
Copy link
Contributor

s0kil commented Sep 21, 2020

We could also allow overwriting, the width and height attributes: https://github.com/mint-lang/mint/blob/master/src/compilers/directives/svg.cr#L18

@s0kil s0kil added feature request language Language feature labels Sep 21, 2020
@s0kil
Copy link
Contributor

s0kil commented Sep 21, 2020

Are those three the most important, or should we add all possible <svg> attributes?
https://developer.mozilla.org/en-US/docs/Web/SVG/Element/svg#Attributes

It seems these six are the most relevant:
preserveAspectRatio, viewBox, height, width, x, y

@gdotdesign
Copy link
Member

Are those three the most important, or should we add all possible <svg> attributes?

Yes we can add other attributes as well, it might come in handy for some use cases.

@s0kil
Copy link
Contributor

s0kil commented Sep 21, 2020

We currently raise an issue, when the width, height, viewBox attributes are not defined on the SVG, is this required?

@gdotdesign
Copy link
Member

gdotdesign commented Sep 21, 2020

We currently raise an issue, when the width, height, viewBox attributes are not defined on the SVG, is this required?

With the current implementation yes they are required because we use them.

I don't see any reason why we could not make them optional (only use them when they are present), and overridable.

I guess I made them required because it is a good practice to have them.

@s0kil
Copy link
Contributor

s0kil commented Sep 23, 2020

const ALERT = @svg(../public/tiger.svg,
  preserveAspectRatio = "xMidYMid meet",
  viewBox = "0 0 100 100",
  width = 100,
  height = 100,
  x = 0,
  y = 0)

@gdotdesign gdotdesign added this to the 0.13.0 milestone Apr 17, 2021
@gdotdesign gdotdesign removed this from the 0.13.0 milestone Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants