Skip to content

fix(progress-bar): remove data url from progress-bar's css styling - #8898

Merged
jelbourn merged 1 commit into
angular:masterfrom
josephperrott:progres-bar
Jan 26, 2018
Merged

fix(progress-bar): remove data url from progress-bar's css styling#8898
jelbourn merged 1 commit into
angular:masterfrom
josephperrott:progres-bar

Conversation

@josephperrott

Copy link
Copy Markdown
Member

Fixes#8876

@googlebotgooglebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 8, 2017
<div class="mat-progress-bar-background mat-progress-bar-element">
<svg width="100%" height="5">
<defs>
<pattern id="circle" x="5" y="0" width="10" height="5" patternUnits="userSpaceOnUse">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AFAIK this ID has to be unique.

Comment threadsrc/lib/progress-bar/progress-bar.ts Outdated
* then placed back into the DOM again. This is necessary as the browser only defines the
* styling of SVG pattern elements as it is placed into the DOM.
*/
private _reappendCircleSvgElement() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not convinced that this is necessary. The progress spinner supports changing the colors without having to re-append the element to the DOM. Are you sure that it doesn't have something to do with the circles not having a unique id?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I think you are right

@josephperrott
josephperrottforce-pushed the progres-bar branch 3 times, most recently from e6cc39d to 97f8482CompareDecember 8, 2017 22:17
<div class="mat-progress-bar-background mat-progress-bar-element">
<svg width="100%" height="5">
<defs>
<pattern id="{{progressbarId}}" x="5" y="0" width="10" height="5" patternUnits="userSpaceOnUse">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use [id] instead of interpolation?

background-size: 10px 4px;
display: none;
svg {
position: absolute;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also comment on why the element is absolute?

@josephperrott

Copy link
Copy Markdown
MemberAuthor

@jelbourn ready for another look when you have a moment.

@jelbournjelbourn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jelbournjelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 25, 2018
@jelbourn
jelbourn merged commit 0f2ac9b into angular:masterJan 26, 2018
@jelbournjelbourn added the target: minor This PR is targeted for the next minor release label Jan 29, 2018
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-botangular-automatic-lock-botBot locked and limited conversation to collaborators Sep 8, 2019
@josephperrott
josephperrott deleted the progres-bar branch March 20, 2020 22:15
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: mergeThe PR is ready for merge by the caretakercla: yesPR author has agreed to Google's Contributor License Agreementtarget: minorThis PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer progress bar "ball" is blocked by CSP due to data url svg

4 participants

@josephperrott@jelbourn@crisbeto@googlebot