Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(progressCircular): fixes scaling issues #5891

Closed
wants to merge 1 commit into from

Conversation

robertmesserle
Copy link
Contributor

@robertmesserle robertmesserle added the needs: review This PR is waiting on review from the team label Nov 23, 2015
height: (100 * getDiameterRatio()) + 'px'
});
circle.children().eq(0).css(toVendorCSS({
transform : $mdUtil.supplant('translate(-50%, -50%) scale( {0} )',[getDiameterRatio()])
Copy link
Contributor

Choose a reason for hiding this comment

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

@robertmesserle - is the translate( ) fixed regardless of the scale value ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, it's a percentage-based translate, so it will center the spinner.

Copy link
Member

Choose a reason for hiding this comment

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

this line could use an explanation of its intent.

@ThomasBurleson ThomasBurleson modified the milestone: 1.0-rc5 Nov 24, 2015
@@ -74,7 +75,7 @@ md-progress-circular {
}


.md-spinner-wrapper.md-mode-indeterminate {
.md-mode-indeterminate .md-spinner-wrapper {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, was there a reason why these were swapped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The DOM structure had to change to make this happen. I added an extra container that could be scaled separate from the outer container.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah gotcha! I missed the space between them or I wouldn't have asked (I thought you literally just swapped the order of the selector 😄).

@topherfangio
Copy link
Contributor

LGTM (assuming tests pass 😄)

@ThomasBurleson ThomasBurleson added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Nov 24, 2015
@ThomasBurleson
Copy link
Contributor

@robertmesserle - CI tests are failing.

@ThomasBurleson ThomasBurleson added needs: tests and removed pr: merge ready This PR is ready for a caretaker to review labels Nov 24, 2015
@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc5, 1.0-rc6 Nov 24, 2015
@robertmesserle robertmesserle force-pushed the issues/4839 branch 2 times, most recently from 50af5b0 to 144d36d Compare November 24, 2015 22:25
@robertmesserle robertmesserle added needs: review This PR is waiting on review from the team and removed needs: tests labels Nov 24, 2015
@ThomasBurleson ThomasBurleson added needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved and removed needs: review This PR is waiting on review from the team labels Nov 25, 2015
@ThomasBurleson
Copy link
Contributor

@robertmesserle - please rebase.

@robertmesserle
Copy link
Contributor Author

@ThomasBurleson Done

@EladBezalel EladBezalel deleted the issues/4839 branch January 15, 2016 23:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants