Skip to content

Use embedded SVGs for carousel icons#30268

Closed
MartijnCuppens wants to merge 3 commits intomainfrom
master-mc-carousel-svg
Closed

Use embedded SVGs for carousel icons#30268
MartijnCuppens wants to merge 3 commits intomainfrom
master-mc-carousel-svg

Conversation

@MartijnCuppens
Copy link
Member

Part of #30052.

@XhmikosR, do you know a way to easily indent the svgs in the example code?

@XhmikosR
Copy link
Member

My main concern with this is that the SVGs shouldn't be in the docs since we don't distribute the docs.

@MartijnCuppens
Copy link
Member Author

My main concern with this is that the SVGs shouldn't be in the docs since we don't distribute the docs.

From my perspective, the SVGs are just markup that are needed to make the carousel work.

@MartijnCuppens MartijnCuppens mentioned this pull request Feb 23, 2020
4 tasks
@XhmikosR
Copy link
Member

XhmikosR commented Feb 23, 2020

From my perspective, the SVGs are just markup that are needed to make the carousel work.

I agree, but people might argue that the SVGs should be included.

I'm not against this, obviously, just trying to think what issues we might face.

@XhmikosR, do you know a way to easily indent the svgs in the example code?

Not sure about this right now, I'll need to play with it and see, because using - doesn't seem to work here.

@ffoodd
Copy link
Member

ffoodd commented Apr 23, 2020

About markup VS distribution, just a reminder that for now our close button is text-based and thus requires to get the markup. I think it's the same.

Apart from that, it'd be very easy to make SVGs downloadable from the docs without distributing them in our package: might be another way around this.

@mdo mdo changed the base branch from master to main June 16, 2020 20:11
@mdo
Copy link
Member

mdo commented Sep 14, 2020

Closing for #31650.

@mdo mdo closed this Sep 14, 2020
@mdo mdo deleted the master-mc-carousel-svg branch September 14, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants