Skip to content

Fix width and height of data svg images in IE10#28883

Merged
XhmikosR merged 1 commit into
twbs:v4-dev-xmrfrom
mort3za:fix-custom-select-in-ie10
Jun 17, 2019
Merged

Fix width and height of data svg images in IE10#28883
XhmikosR merged 1 commit into
twbs:v4-dev-xmrfrom
mort3za:fix-custom-select-in-ie10

Conversation

@mort3za

@mort3za mort3za commented Jun 9, 2019

Copy link
Copy Markdown

Inline svg of custom-select is broken in IE10 because there is no width and height attributes.
Tested on a real IE10 in browserstack, it works.

@mort3za mort3za requested a review from a team as a code owner June 9, 2019 10:52
@XhmikosR XhmikosR changed the base branch from v4-dev to master June 9, 2019 11:18
@XhmikosR XhmikosR changed the base branch from master to v4-dev June 9, 2019 11:19
@XhmikosR

XhmikosR commented Jun 9, 2019

Copy link
Copy Markdown
Member

If this lands it needs to target master and also make sure all other SVGs don't have this issue.

@XhmikosR XhmikosR added the css label Jun 9, 2019
@XhmikosR XhmikosR changed the base branch from v4-dev to master June 9, 2019 12:42
@XhmikosR

XhmikosR commented Jun 9, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Changed the target branch so that it can't be merged blindly. See my message above.

@MartijnCuppens

MartijnCuppens commented Jun 11, 2019

Copy link
Copy Markdown
Member

He did a PR targeting the master branch here: #28882. I asked him to do target the v4-dev branch because we're dropping IE10 support anyway. If you prefer the master branch to be targeted, you can close this PR and reopen #28882, @XhmikosR.

@XhmikosR XhmikosR changed the base branch from master to v4-dev-xmr June 11, 2019 10:30
@XhmikosR

XhmikosR commented Jun 11, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Switched this to v4-dev-xmr.

But yeah, master is moot indeed.

@XhmikosR XhmikosR added the v4 label Jun 11, 2019
@XhmikosR

XhmikosR commented Jun 11, 2019

Copy link
Copy Markdown
Member

I'd still appreciate someone checking the other inline SVGs, because IIRC we don't use width/height.

@mort3za

mort3za commented Jun 11, 2019

Copy link
Copy Markdown
Author

@XhmikosR I'll check them today

@mdo

mdo commented Jun 11, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Going to make a note of this for my v5 forms branch at #28450 to at least confirm sizing works in IE11+, Edge, etc as I'm intending.

@MartijnCuppens

MartijnCuppens commented Jun 11, 2019

Copy link
Copy Markdown
Member

Only IE10 seems to have this is issue afaik.

@mdo

mdo commented Jun 11, 2019

Copy link
Copy Markdown
Member

Yeah, it just reminded me that I don't recall if there are any major hiccups and we're going all in on custom forms in v5. With new SVGs, I want to make sure I'm keeping things in mind and can document known issues to help folks customize.

@mort3za mort3za force-pushed the fix-custom-select-in-ie10 branch from dd2edd6 to 9c2f144 Compare June 11, 2019 19:43
@mort3za

mort3za commented Jun 11, 2019

Copy link
Copy Markdown
Author

Searched entire project, only variables.scss has such inline SVG images. Pull request updated.

@XhmikosR

XhmikosR commented Jun 13, 2019

Copy link
Copy Markdown
Member

@twbs/css-review: can you review one final time please?

@XhmikosR

This comment has been minimized.

Sign in to view
@XhmikosR XhmikosR force-pushed the fix-custom-select-in-ie10 branch from 9c2f144 to 9305099 Compare June 17, 2019 14:00
@XhmikosR XhmikosR merged commit af25600 into twbs:v4-dev-xmr Jun 17, 2019
XhmikosR pushed a commit that referenced this pull request Jun 18, 2019
@mort3za @XhmikosR
Fix width and height of data svg images in IE10 (#28883)
bbc4a2b
@XhmikosR XhmikosR changed the title fix custom-select-indicator in IE10 Fix width and height of data svg images in IE10 Jun 18, 2019
XhmikosR pushed a commit that referenced this pull request Jun 18, 2019
@mort3za @XhmikosR
Fix width and height of data svg images in IE10 (#28883)
1bdeab8
@mdo mdo mentioned this pull request Jul 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MartijnCuppens MartijnCuppens MartijnCuppens approved these changes

+1 more reviewer

@ysds ysds ysds approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

css v4

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mort3za @XhmikosR @MartijnCuppens @mdo @ysds